From 2651cd4abaebbe84c3ed762ce3bf99f28ab86122 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 2 Jan 2025 16:36:36 -0500
Subject: [PATCH 01/60] initial approach, from domain.py, not quite working

---
 src/registrar/forms/domain_request_wizard.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 289b3da0b..45bd575f9 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -296,6 +296,14 @@ class OrganizationContactForm(RegistrarForm):
         label="Urbanization (required for Puerto Rico only)",
     )
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        # Set initial value for federal agency combo box and specify combobox template
+        if self.domain_request and self.domain_request.federal_agency:
+            self.fields["federal_agency"].initial = self.domain_request.federal_agency
+            self.fields["federal_agency"].widget.attrs["data-default-value"] = self.domain_request.federal_agency.pk
+        self.fields["federal_agency"].widget.template_name = "django/forms/widgets/combobox.html",
+
     def clean_federal_agency(self):
         """Require something to be selected when this is a federal agency."""
         federal_agency = self.cleaned_data.get("federal_agency", None)

From f359a636d02103c0dd5421a5c25be4314fbc3a27 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 6 Jan 2025 13:45:37 -0500
Subject: [PATCH 02/60] combobox in domain request organization contact

---
 src/registrar/forms/domain_request_wizard.py | 17 ++++++++++++++---
 src/registrar/forms/utility/combobox.py      |  5 +++++
 2 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 src/registrar/forms/utility/combobox.py

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 45bd575f9..f58d62c0e 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -7,6 +7,7 @@ from django import forms
 from django.core.validators import RegexValidator, MaxLengthValidator
 from django.utils.safestring import mark_safe
 
+from registrar.forms.utility.combobox import ComboboxWidget
 from registrar.forms.utility.wizard_form_helper import (
     RegistrarForm,
     RegistrarFormSet,
@@ -257,6 +258,7 @@ class OrganizationContactForm(RegistrarForm):
         required=False,
         queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies),
         empty_label="--Select--",
+        widget=ComboboxWidget,
     )
     organization_name = forms.CharField(
         label="Organization name",
@@ -298,11 +300,20 @@ class OrganizationContactForm(RegistrarForm):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        # Set initial value for federal agency combo box and specify combobox template
-        if self.domain_request and self.domain_request.federal_agency:
+
+        # Initialize federal_agency combobox widget
+        # Domain requests forms have prefix associated with step
+        prefix = kwargs.get("prefix", "")
+        prefixed_name = f"{prefix}-federal_agency" if prefix else "federal_agency"
+
+        # For combobox widget, need to set the data-default-value to selected value
+        if self.is_bound and self.data.get(prefixed_name):
+            # If form is bound (from a POST), use submitted value
+            self.fields["federal_agency"].widget.attrs["data-default-value"] = self.data.get(prefixed_name)
+        elif self.domain_request and self.domain_request.federal_agency:
+            # If form is not bound, set initial
             self.fields["federal_agency"].initial = self.domain_request.federal_agency
             self.fields["federal_agency"].widget.attrs["data-default-value"] = self.domain_request.federal_agency.pk
-        self.fields["federal_agency"].widget.template_name = "django/forms/widgets/combobox.html",
 
     def clean_federal_agency(self):
         """Require something to be selected when this is a federal agency."""
diff --git a/src/registrar/forms/utility/combobox.py b/src/registrar/forms/utility/combobox.py
new file mode 100644
index 000000000..b7db16ccc
--- /dev/null
+++ b/src/registrar/forms/utility/combobox.py
@@ -0,0 +1,5 @@
+from django.forms import Select
+
+
+class ComboboxWidget(Select):
+    template_name = "django/forms/widgets/combobox.html"
\ No newline at end of file

From 4480e3255375120f3ca6612c0f15309ebd57aa9a Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 6 Jan 2025 14:15:13 -0500
Subject: [PATCH 03/60] updated combobox widget to set proper
 data-default-value and set domain suborganization form to use new combobox
 widget

---
 src/registrar/forms/domain.py                  | 18 +++---------------
 src/registrar/forms/domain_request_wizard.py   | 17 -----------------
 .../django/forms/widgets/combobox.html         |  1 +
 3 files changed, 4 insertions(+), 32 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index b43d91a58..1e4068125 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -4,6 +4,7 @@ import logging
 from django import forms
 from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator, MaxLengthValidator
 from django.forms import formset_factory
+from registrar.forms.utility.combobox import ComboboxWidget
 from registrar.models import DomainRequest, FederalAgency
 from phonenumber_field.widgets import RegionalPhoneNumberWidget
 from registrar.models.suborganization import Suborganization
@@ -161,9 +162,10 @@ class DomainSuborganizationForm(forms.ModelForm):
     """Form for updating the suborganization"""
 
     sub_organization = forms.ModelChoiceField(
+        label = "Suborganization name",
         queryset=Suborganization.objects.none(),
         required=False,
-        widget=forms.Select(),
+        widget=ComboboxWidget,
     )
 
     class Meta:
@@ -178,20 +180,6 @@ class DomainSuborganizationForm(forms.ModelForm):
         portfolio = self.instance.portfolio if self.instance else None
         self.fields["sub_organization"].queryset = Suborganization.objects.filter(portfolio=portfolio)
 
-        # Set initial value
-        if self.instance and self.instance.sub_organization:
-            self.fields["sub_organization"].initial = self.instance.sub_organization
-
-        # Set custom form label
-        self.fields["sub_organization"].label = "Suborganization name"
-
-        # Use the combobox rather than the regular select widget
-        self.fields["sub_organization"].widget.template_name = "django/forms/widgets/combobox.html"
-
-        # Set data-default-value attribute
-        if self.instance and self.instance.sub_organization:
-            self.fields["sub_organization"].widget.attrs["data-default-value"] = self.instance.sub_organization.pk
-
 
 class BaseNameserverFormset(forms.BaseFormSet):
     def clean(self):
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index f58d62c0e..e090ac8d2 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -298,23 +298,6 @@ class OrganizationContactForm(RegistrarForm):
         label="Urbanization (required for Puerto Rico only)",
     )
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-        # Initialize federal_agency combobox widget
-        # Domain requests forms have prefix associated with step
-        prefix = kwargs.get("prefix", "")
-        prefixed_name = f"{prefix}-federal_agency" if prefix else "federal_agency"
-
-        # For combobox widget, need to set the data-default-value to selected value
-        if self.is_bound and self.data.get(prefixed_name):
-            # If form is bound (from a POST), use submitted value
-            self.fields["federal_agency"].widget.attrs["data-default-value"] = self.data.get(prefixed_name)
-        elif self.domain_request and self.domain_request.federal_agency:
-            # If form is not bound, set initial
-            self.fields["federal_agency"].initial = self.domain_request.federal_agency
-            self.fields["federal_agency"].widget.attrs["data-default-value"] = self.domain_request.federal_agency.pk
-
     def clean_federal_agency(self):
         """Require something to be selected when this is a federal agency."""
         federal_agency = self.cleaned_data.get("federal_agency", None)
diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html
index 7ff31945b..4fe796347 100644
--- a/src/registrar/templates/django/forms/widgets/combobox.html
+++ b/src/registrar/templates/django/forms/widgets/combobox.html
@@ -11,6 +11,7 @@ for now we just carry the attribute to both the parent element and the select.
         {{ name }}="{{ value }}"
     {% endif %}
 {% endfor %}
+data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}"
 >
     {% include "django/forms/widgets/select.html" %}
 </div>

From 3146dc07fadd3d9b92e5932b7964f57943cb271a Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 6 Jan 2025 14:22:36 -0500
Subject: [PATCH 04/60] applied widget to state territory

---
 src/registrar/forms/domain_request_wizard.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index e090ac8d2..fcba68de8 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -282,6 +282,7 @@ class OrganizationContactForm(RegistrarForm):
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
+        widget=ComboboxWidget,
     )
     zipcode = forms.CharField(
         label="Zip code",

From 560baed2e1e2eb95aa48364b2e3673084814a741 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 6 Jan 2025 14:39:44 -0500
Subject: [PATCH 05/60] linted

---
 src/registrar/forms/domain.py           | 2 +-
 src/registrar/forms/utility/combobox.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 1e4068125..87a52d142 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -162,7 +162,7 @@ class DomainSuborganizationForm(forms.ModelForm):
     """Form for updating the suborganization"""
 
     sub_organization = forms.ModelChoiceField(
-        label = "Suborganization name",
+        label="Suborganization name",
         queryset=Suborganization.objects.none(),
         required=False,
         widget=ComboboxWidget,
diff --git a/src/registrar/forms/utility/combobox.py b/src/registrar/forms/utility/combobox.py
index b7db16ccc..277aec4f3 100644
--- a/src/registrar/forms/utility/combobox.py
+++ b/src/registrar/forms/utility/combobox.py
@@ -2,4 +2,4 @@ from django.forms import Select
 
 
 class ComboboxWidget(Select):
-    template_name = "django/forms/widgets/combobox.html"
\ No newline at end of file
+    template_name = "django/forms/widgets/combobox.html"

From bb5f61fd989d3ac6588a1f7a4e30f01ac99556fe Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 10 Jan 2025 06:50:36 -0500
Subject: [PATCH 06/60] updated behavior of combobox, added state territory
 combobox

---
 src/registrar/forms/domain_request_wizard.py             | 1 +
 src/registrar/templates/django/forms/widgets/select.html | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index fcba68de8..c38eb01a2 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -57,6 +57,7 @@ class RequestingEntityForm(RegistrarForm):
         label="State, territory, or military post",
         required=False,
         choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices,
+        widget=ComboboxWidget,
     )
 
     def __init__(self, *args, **kwargs):
diff --git a/src/registrar/templates/django/forms/widgets/select.html b/src/registrar/templates/django/forms/widgets/select.html
index cc62eb91d..86ca4ddcc 100644
--- a/src/registrar/templates/django/forms/widgets/select.html
+++ b/src/registrar/templates/django/forms/widgets/select.html
@@ -3,6 +3,7 @@
     {# hint: spacing in the class string matters #}
     class="usa-select{% if classes %} {{ classes }}{% endif %}"
     {% include "django/forms/widgets/attrs.html" %}
+    data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}"
 >
   {% for group, options, index in widget.optgroups %}
     {% if group %}<optgroup label="{{ group }}">{% endif %}

From 0e2d62f86f6cee2d63f0813b59f4a4852ce4d729 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 10 Jan 2025 06:59:26 -0500
Subject: [PATCH 07/60] small improvement

---
 src/registrar/templates/django/forms/widgets/combobox.html | 2 +-
 src/registrar/templates/django/forms/widgets/select.html   | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/registrar/templates/django/forms/widgets/combobox.html b/src/registrar/templates/django/forms/widgets/combobox.html
index 4fe796347..02cd4e35e 100644
--- a/src/registrar/templates/django/forms/widgets/combobox.html
+++ b/src/registrar/templates/django/forms/widgets/combobox.html
@@ -13,5 +13,5 @@ for now we just carry the attribute to both the parent element and the select.
 {% endfor %}
 data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}"
 >
-    {% include "django/forms/widgets/select.html" %}
+    {% include "django/forms/widgets/select.html" with is_combobox=True %}
 </div>
diff --git a/src/registrar/templates/django/forms/widgets/select.html b/src/registrar/templates/django/forms/widgets/select.html
index 86ca4ddcc..db6deafe2 100644
--- a/src/registrar/templates/django/forms/widgets/select.html
+++ b/src/registrar/templates/django/forms/widgets/select.html
@@ -3,7 +3,9 @@
     {# hint: spacing in the class string matters #}
     class="usa-select{% if classes %} {{ classes }}{% endif %}"
     {% include "django/forms/widgets/attrs.html" %}
-    data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}"
+    {% if is_combobox %}
+      data-default-value="{% for group_name, group_choices, group_index in widget.optgroups %}{% for option in group_choices %}{% if option.selected %}{{ option.value }}{% endif %}{% endfor %}{% endfor %}"
+    {% endif %}
 >
   {% for group, options, index in widget.optgroups %}
     {% if group %}<optgroup label="{{ group }}">{% endif %}

From 6283cad872ef79472ba6dcf60e33cc1479fa90e7 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 10 Jan 2025 07:22:38 -0500
Subject: [PATCH 08/60] state territory in org name address domain form

---
 src/registrar/forms/domain.py | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 87a52d142..6cf6416c1 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -457,6 +457,17 @@ class DomainOrgNameAddressForm(forms.ModelForm):
         },
     )
 
+    state_territory = forms.ChoiceField(
+        label="State, territory, or military post",
+        required=True,
+        choices=DomainInformation.StateTerritoryChoices.choices,
+        widget=ComboboxWidget(
+            attrs={
+                "required": True,
+            }
+        ),
+    )
+
     class Meta:
         model = DomainInformation
         fields = [
@@ -479,20 +490,10 @@ class DomainOrgNameAddressForm(forms.ModelForm):
             },
         }
         widgets = {
-            # We need to set the required attributed for State/territory
-            # because for this fields we are creating an individual
-            # instance of the Select. For the other fields we use the for loop to set
-            # the class's required attribute to true.
             "organization_name": forms.TextInput,
             "address_line1": forms.TextInput,
             "address_line2": forms.TextInput,
             "city": forms.TextInput,
-            "state_territory": forms.Select(
-                attrs={
-                    "required": True,
-                },
-                choices=DomainInformation.StateTerritoryChoices.choices,
-            ),
             "urbanization": forms.TextInput,
         }
 

From d41174547d7ca5fd11bc0929103a58bb40c137bc Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 10 Jan 2025 07:35:44 -0500
Subject: [PATCH 09/60] fixing test_export again

---
 src/registrar/tests/test_reports.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index 4a41238c7..93c07df13 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -900,6 +900,7 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib):
         # spaces and leading/trailing whitespace
         csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip()
         expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip()
+        self.maxDiff=None
         self.assertEqual(csv_content, expected_content)
 
 

From b4505b38630eb2df71d33d2c50bcc960995cc790 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 10 Jan 2025 07:38:56 -0500
Subject: [PATCH 10/60] lint

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

diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index 93c07df13..b11500ea9 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -900,7 +900,7 @@ class MemberExportTest(MockDbForIndividualTests, MockEppLib):
         # spaces and leading/trailing whitespace
         csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip()
         expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip()
-        self.maxDiff=None
+        self.maxDiff = None
         self.assertEqual(csv_content, expected_content)
 
 

From ae6c461ddb33f67c0b7bea042a4ca6594e30e669 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 13 Jan 2025 08:00:02 -0500
Subject: [PATCH 11/60] added federal agency on domain page, and suborg on
 requesting entity page

---
 src/registrar/forms/domain.py                | 9 +++++++++
 src/registrar/forms/domain_request_wizard.py | 1 +
 2 files changed, 10 insertions(+)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 6cf6416c1..dd8c719cd 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -444,6 +444,15 @@ class DomainSecurityEmailForm(forms.Form):
 class DomainOrgNameAddressForm(forms.ModelForm):
     """Form for updating the organization name and mailing address."""
 
+    # for federal agencies we also want to know the top-level agency.
+    excluded_agencies = ["gov Administration", "Non-Federal Agency"]
+    federal_agency = forms.ModelChoiceField(
+        label="Federal agency",
+        required=False,
+        queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies),
+        empty_label="--Select--",
+        widget=ComboboxWidget,
+    )
     zipcode = forms.CharField(
         label="Zip code",
         validators=[
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 898a02d58..2365d323d 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -45,6 +45,7 @@ class RequestingEntityForm(RegistrarForm):
         required=False,
         queryset=Suborganization.objects.none(),
         empty_label="--Select--",
+        widget=ComboboxWidget,
     )
     requested_suborganization = forms.CharField(
         label="Requested suborganization",

From d54e57b0840b01d0760669e143916ebe522e5a91 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 13 Jan 2025 08:15:50 -0500
Subject: [PATCH 12/60] update federal agency drop down not to have any
 exclusions

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

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index dd8c719cd..7089409de 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -445,11 +445,10 @@ class DomainOrgNameAddressForm(forms.ModelForm):
     """Form for updating the organization name and mailing address."""
 
     # for federal agencies we also want to know the top-level agency.
-    excluded_agencies = ["gov Administration", "Non-Federal Agency"]
     federal_agency = forms.ModelChoiceField(
         label="Federal agency",
         required=False,
-        queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies),
+        queryset=FederalAgency.objects.all(),
         empty_label="--Select--",
         widget=ComboboxWidget,
     )

From 828c47de4528fc7bec86639087f91cfe628e835a Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Mon, 13 Jan 2025 15:06:44 -0700
Subject: [PATCH 13/60] Better table + simplify get_or_create function

---
 src/registrar/admin.py         |  3 ++
 src/registrar/models/domain.py | 68 ++++++++++++++--------------------
 2 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 849cb6100..ed0b776ce 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -3835,6 +3835,9 @@ class PublicContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
 
     change_form_template = "django/admin/email_clipboard_change_form.html"
     autocomplete_fields = ["domain"]
+    list_display = ("domain", "email", "name", "contact_type", "id")
+    search_fields = ["email", "name", "id"]
+    search_help_text = "Search by email, name or id."
 
     def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
         if extra_context is None:
diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 6bd8278a1..cb912cb37 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -4,9 +4,9 @@ import ipaddress
 import re
 from datetime import date, timedelta
 from typing import Optional
+from django.db import transaction
 from django_fsm import FSMField, transition, TransitionNotAllowed  # type: ignore
-
-from django.db import models
+from django.db import models, IntegrityError
 from django.utils import timezone
 from typing import Any
 from registrar.models.host import Host
@@ -2077,49 +2077,35 @@ class Domain(TimeStampedModel, DomainHelper):
     def _get_or_create_public_contact(self, public_contact: PublicContact):
         """Tries to find a PublicContact object in our DB.
         If it can't, it'll create it. Returns PublicContact"""
-        db_contact = PublicContact.objects.filter(
-            registry_id=public_contact.registry_id,
-            contact_type=public_contact.contact_type,
-            domain=self,
-        )
-
-        # If we find duplicates, log it and delete the oldest ones.
-        if db_contact.count() > 1:
-            logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.")
-
-            newest_duplicate = db_contact.order_by("-created_at").first()
-
-            duplicates_to_delete = db_contact.exclude(id=newest_duplicate.id)  # type: ignore
-
-            # Delete all duplicates
-            duplicates_to_delete.delete()
-
-            # Do a second filter to grab the latest content
-            db_contact = PublicContact.objects.filter(
+        try:
+            with transaction.atomic():
+                contact, _ = PublicContact.objects.get_or_create(
+                    registry_id=public_contact.registry_id,
+                    contact_type=public_contact.contact_type,
+                    domain=self,
+                    defaults={
+                        "email": public_contact.email,
+                        "voice": public_contact.voice,
+                        "fax": public_contact.fax,
+                        "name": public_contact.name,
+                        "org": public_contact.org,
+                        "pw": public_contact.pw,
+                        "city": public_contact.city,
+                        "pc": public_contact.pc,
+                        "cc": public_contact.cc,
+                        "sp": public_contact.sp,
+                        "street1": public_contact.street1,
+                        "street2": public_contact.street2,
+                        "street3": public_contact.street3,
+                    }
+                )
+        except IntegrityError:
+            contact = PublicContact.objects.get(
                 registry_id=public_contact.registry_id,
                 contact_type=public_contact.contact_type,
                 domain=self,
             )
-
-        # Save to DB if it doesn't exist already.
-        if db_contact.count() == 0:
-            # Doesn't run custom save logic, just saves to DB
-            public_contact.save(skip_epp_save=True)
-            logger.info(f"Created a new PublicContact: {public_contact}")
-            # Append the item we just created
-            return public_contact
-
-        existing_contact = db_contact.get()
-
-        # Does the item we're grabbing match what we have in our DB?
-        if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id:
-            existing_contact.delete()
-            public_contact.save()
-            logger.warning("Requested PublicContact is out of sync " "with DB.")
-            return public_contact
-
-        # If it already exists, we can assume that the DB instance was updated during set, so we should just use that.
-        return existing_contact
+        return contact
 
     def _registrant_to_public_contact(self, registry_id: str):
         """EPPLib returns the registrant as a string,

From e2486c127da6627b4bdcdac15bfeeaf81597d687 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 14 Jan 2025 08:42:22 -0500
Subject: [PATCH 14/60] organization edit form - added state territory combobox

---
 src/registrar/forms/portfolio.py | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 0a8c4d623..4e2e7bdf1 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -6,6 +6,7 @@ from django.core.validators import RegexValidator
 from django.core.validators import MaxLengthValidator
 from django.utils.safestring import mark_safe
 
+from registrar.forms.utility.combobox import ComboboxWidget
 from registrar.models import (
     PortfolioInvitation,
     UserPortfolioPermission,
@@ -33,6 +34,12 @@ class PortfolioOrgAddressForm(forms.ModelForm):
             "required": "Enter a 5-digit or 9-digit zip code, like 12345 or 12345-6789.",
         },
     )
+    state_territory = forms.ChoiceField(
+        label="State, territory, or military post",
+        required=True,
+        choices=DomainInformation.StateTerritoryChoices.choices,
+        widget=ComboboxWidget,
+    )
 
     class Meta:
         model = Portfolio
@@ -53,19 +60,9 @@ class PortfolioOrgAddressForm(forms.ModelForm):
             "zipcode": {"required": "Enter a 5-digit or 9-digit zip code, like 12345 or 12345-6789."},
         }
         widgets = {
-            # We need to set the required attributed for State/territory
-            # because for this fields we are creating an individual
-            # instance of the Select. For the other fields we use the for loop to set
-            # the class's required attribute to true.
             "address_line1": forms.TextInput,
             "address_line2": forms.TextInput,
             "city": forms.TextInput,
-            "state_territory": forms.Select(
-                attrs={
-                    "required": True,
-                },
-                choices=DomainInformation.StateTerritoryChoices.choices,
-            ),
             # "urbanization": forms.TextInput,
         }
 

From c0c9817c99459a0ecfef76a59fe12a9ea9483bfa Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 14 Jan 2025 10:33:00 -0700
Subject: [PATCH 15/60] Update admin.py

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

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index ed0b776ce..8773f7ef8 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -3835,9 +3835,9 @@ class PublicContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
 
     change_form_template = "django/admin/email_clipboard_change_form.html"
     autocomplete_fields = ["domain"]
-    list_display = ("domain", "email", "name", "contact_type", "id")
-    search_fields = ["email", "name", "id"]
-    search_help_text = "Search by email, name or id."
+    list_display = ("name", "contact_type", "email", "domain", "registry_id")
+    search_fields = ["email", "name", "registry_id"]
+    search_help_text = "Search by email, name or registry id."
 
     def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
         if extra_context is None:

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 16/60] 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 17/60] 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 f86a065a607e1bbc0d3705193559ded32891d3d3 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 14 Jan 2025 15:04:56 -0700
Subject: [PATCH 18/60] Update domain.py

---
 src/registrar/models/domain.py | 85 +++++++++++++++++++++++-----------
 1 file changed, 57 insertions(+), 28 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index cb912cb37..5e186efff 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1329,7 +1329,7 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def get_default_administrative_contact(self):
         """Gets the default administrative contact."""
-        logger.info("get_default_security_contact() -> Adding administrative security contact")
+        logger.info("get_default_administrative_contact() -> Adding administrative security contact")
         contact = PublicContact.get_default_administrative()
         contact.domain = self
         return contact
@@ -1848,7 +1848,6 @@ class Domain(TimeStampedModel, DomainHelper):
         """
         try:
             self._add_missing_contacts_if_unknown(cleaned)
-
         except Exception as e:
             logger.error(
                 "%s couldn't _add_missing_contacts_if_unknown, error was %s."
@@ -1866,7 +1865,6 @@ class Domain(TimeStampedModel, DomainHelper):
         is in an UNKNOWN state, that is an error state)
         Note: The transition state change happens at the end of the function
         """
-
         missingAdmin = True
         missingSecurity = True
         missingTech = True
@@ -1890,6 +1888,11 @@ class Domain(TimeStampedModel, DomainHelper):
             if missingTech:
                 technical_contact = self.get_default_technical_contact()
                 technical_contact.save()
+            
+            logger.info(
+                "_add_missing_contacts_if_unknown => " 
+                f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}"
+            )
 
     def _fetch_cache(self, fetch_hosts=False, fetch_contacts=False):
         """Contact registry for info about a domain."""
@@ -2077,35 +2080,61 @@ class Domain(TimeStampedModel, DomainHelper):
     def _get_or_create_public_contact(self, public_contact: PublicContact):
         """Tries to find a PublicContact object in our DB.
         If it can't, it'll create it. Returns PublicContact"""
-        try:
-            with transaction.atomic():
-                contact, _ = PublicContact.objects.get_or_create(
-                    registry_id=public_contact.registry_id,
-                    contact_type=public_contact.contact_type,
-                    domain=self,
-                    defaults={
-                        "email": public_contact.email,
-                        "voice": public_contact.voice,
-                        "fax": public_contact.fax,
-                        "name": public_contact.name,
-                        "org": public_contact.org,
-                        "pw": public_contact.pw,
-                        "city": public_contact.city,
-                        "pc": public_contact.pc,
-                        "cc": public_contact.cc,
-                        "sp": public_contact.sp,
-                        "street1": public_contact.street1,
-                        "street2": public_contact.street2,
-                        "street3": public_contact.street3,
-                    }
-                )
-        except IntegrityError:
-            contact = PublicContact.objects.get(
+        db_contact = PublicContact.objects.filter(
+            registry_id=public_contact.registry_id,
+            contact_type=public_contact.contact_type,
+            domain=self,
+        )
+
+        # If we find duplicates, log it and delete the oldest ones.
+        if db_contact.count() > 1:
+            logger.warning("_get_or_create_public_contact() -> Duplicate contacts found. Deleting duplicate.")
+
+            newest_duplicate = db_contact.order_by("-created_at").first()
+
+            duplicates_to_delete = db_contact.exclude(id=newest_duplicate.id)  # type: ignore
+
+            # Delete all duplicates
+            duplicates_to_delete.delete()
+
+            # Do a second filter to grab the latest content
+            db_contact = PublicContact.objects.filter(
                 registry_id=public_contact.registry_id,
                 contact_type=public_contact.contact_type,
                 domain=self,
             )
-        return contact
+
+        # Save to DB if it doesn't exist already.
+        if db_contact.count() == 0:
+            # Doesn't run custom save logic, just saves to DB
+            try:
+                public_contact.save(skip_epp_save=True)
+                logger.info(f"Created a new PublicContact: {public_contact}")
+            except IntegrityError as err:
+                logger.error(
+                    "_get_or_create_public_contact() => tried to create a duplicate public contact: "
+                    f"{err}", exc_info=True
+                )
+                return PublicContact.objects.get(
+                    registry_id=public_contact.registry_id,
+                    contact_type=public_contact.contact_type,
+                    domain=self,
+                )
+
+            # Append the item we just created
+            return public_contact
+
+        existing_contact = db_contact.get()
+
+        # Does the item we're grabbing match what we have in our DB?
+        if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id:
+            existing_contact.delete()
+            public_contact.save()
+            logger.warning("Requested PublicContact is out of sync " "with DB.")
+            return public_contact
+
+        # If it already exists, we can assume that the DB instance was updated during set, so we should just use that.
+        return existing_contact
 
     def _registrant_to_public_contact(self, registry_id: str):
         """EPPLib returns the registrant as a string,

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 19/60] 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 ea509f63efc2880052900ab80b587106200b453b Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 14 Jan 2025 18:03:24 -0500
Subject: [PATCH 20/60] uswds edits to combobox, default values for comboboxes,
 cleanup of combobox js

---
 src/registrar/assets/js/uswds-edited.js       | 14 ++++++--
 .../assets/src/js/getgov/combobox.js          | 36 -------------------
 src/registrar/forms/domain.py                 | 15 ++++----
 src/registrar/forms/portfolio.py              |  8 ++---
 4 files changed, 21 insertions(+), 52 deletions(-)

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index f59417b41..b597f2d2b 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -29,6 +29,8 @@
  *  - tooltip dynamic content updated to include nested element (for better sizing control)
  *  - modal exposed to window to be accessible in other js files
  *  - fixed bug in createHeaderButton which added newlines to header button tooltips
+ *  - modified combobox to allow for blank values in list
+ *  - modified aria label for X button in combobox to reflect modified behavior of button
  */
 
 if ("document" in window.self) {
@@ -1218,9 +1220,11 @@ const enhanceComboBox = _comboBoxEl => {
     input.setAttribute(key, value);
   }));
   comboBoxEl.insertAdjacentElement("beforeend", input);
+  // DOTGOV - modified the aria-label of the clear input button to Reset selection to reflect changed button behavior
+  // <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Clear the select contents">&nbsp;</button>
   comboBoxEl.insertAdjacentHTML("beforeend", Sanitizer.escapeHTML`
     <span class="${CLEAR_INPUT_BUTTON_WRAPPER_CLASS}" tabindex="-1">
-        <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Clear the select contents">&nbsp;</button>
+        <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Reset selection">&nbsp;</button>
       </span>
       <span class="${INPUT_BUTTON_SEPARATOR_CLASS}">&nbsp;</span>
       <span class="${TOGGLE_LIST_BUTTON_WRAPPER_CLASS}" tabindex="-1">
@@ -1356,8 +1360,12 @@ const displayList = el => {
   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
     const optionEl = selectEl.options[i];
     const optionId = `${listOptionBaseId}${options.length}`;
-    if (optionEl.value && (disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
-      if (selectEl.value && optionEl.value === selectEl.value) {
+    // DOTGOV: modified combobox to allow for options with blank value
+    //if (optionEl.value && (disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
+    if ((disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
+      // DOTGOV: modified combobox to allow blank option value selections to be considered selected
+      //if (selectEl.value && optionEl.value === selectEl.value) {
+      if (selectEl.value && optionEl.value === selectEl.value || (!selectEl.value && !optionEl.value)) {
         selectedItemId = optionId;
       }
       if (disableFiltering && !firstFoundId && regex.test(optionEl.text)) {
diff --git a/src/registrar/assets/src/js/getgov/combobox.js b/src/registrar/assets/src/js/getgov/combobox.js
index 36b7aa0ad..95c5ff7e8 100644
--- a/src/registrar/assets/src/js/getgov/combobox.js
+++ b/src/registrar/assets/src/js/getgov/combobox.js
@@ -28,19 +28,6 @@ export function loadInitialValuesForComboBoxes() {
       // Override the default clear button behavior such that it no longer clears the input,
       // it just resets to the data-initial-value.
       // Due to the nature of how uswds works, this is slightly hacky.
-      // Use a MutationObserver to watch for changes in the dropdown list
-      const dropdownList = comboBox.querySelector(`#${input.id}--list`);
-      const observer = new MutationObserver(function(mutations) {
-          mutations.forEach(function(mutation) {
-              if (mutation.type === "childList") {
-                addBlankOption(clearInputButton, dropdownList, initialValue);
-              }
-          });
-      });
-
-      // Configure the observer to watch for changes in the dropdown list
-      const config = { childList: true, subtree: true };
-      observer.observe(dropdownList, config);
 
       // Input event listener to detect typing
       input.addEventListener("input", () => {
@@ -87,27 +74,4 @@ export function loadInitialValuesForComboBoxes() {
       showElement(clearInputButton)
     }
   }
-
-  function addBlankOption(clearInputButton, dropdownList, initialValue) {
-    if (dropdownList && !dropdownList.querySelector('[data-value=""]') && !isTyping) {
-        const blankOption = document.createElement("li");
-        blankOption.setAttribute("role", "option");
-        blankOption.setAttribute("data-value", "");
-        blankOption.classList.add("usa-combo-box__list-option");
-        if (!initialValue){
-          blankOption.classList.add("usa-combo-box__list-option--selected")
-        }
-        blankOption.textContent = "⎯";
-
-        dropdownList.insertBefore(blankOption, dropdownList.firstChild);
-        blankOption.addEventListener("click", (e) => {
-          e.preventDefault();
-          e.stopPropagation();
-          overrideDefaultClearButton = false;
-          // Trigger the default clear behavior
-          clearInputButton.click();
-          overrideDefaultClearButton = true;
-        });
-    }
-  }
 }
diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 7089409de..f577ed0ab 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -164,6 +164,7 @@ class DomainSuborganizationForm(forms.ModelForm):
     sub_organization = forms.ModelChoiceField(
         label="Suborganization name",
         queryset=Suborganization.objects.none(),
+        empty_label="⎯ (No suborganization)",
         required=False,
         widget=ComboboxWidget,
     )
@@ -468,12 +469,11 @@ class DomainOrgNameAddressForm(forms.ModelForm):
     state_territory = forms.ChoiceField(
         label="State, territory, or military post",
         required=True,
-        choices=DomainInformation.StateTerritoryChoices.choices,
-        widget=ComboboxWidget(
-            attrs={
-                "required": True,
-            }
-        ),
+        choices=[("", "--Select--")] + DomainInformation.StateTerritoryChoices.choices,
+        error_messages={
+            "required": ("Select the state, territory, or military post where your organization is located.")
+        },
+        widget=ComboboxWidget(),
     )
 
     class Meta:
@@ -493,9 +493,6 @@ class DomainOrgNameAddressForm(forms.ModelForm):
             "organization_name": {"required": "Enter the name of your organization."},
             "address_line1": {"required": "Enter the street address of your organization."},
             "city": {"required": "Enter the city where your organization is located."},
-            "state_territory": {
-                "required": "Select the state, territory, or military post where your organization is located."
-            },
         }
         widgets = {
             "organization_name": forms.TextInput,
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 4e2e7bdf1..13d956fe4 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -37,7 +37,10 @@ class PortfolioOrgAddressForm(forms.ModelForm):
     state_territory = forms.ChoiceField(
         label="State, territory, or military post",
         required=True,
-        choices=DomainInformation.StateTerritoryChoices.choices,
+        choices=[("", "--Select--")] + DomainInformation.StateTerritoryChoices.choices,
+        error_messages={
+            "required": ("Select the state, territory, or military post where your organization is located.")
+        },
         widget=ComboboxWidget,
     )
 
@@ -54,9 +57,6 @@ class PortfolioOrgAddressForm(forms.ModelForm):
         error_messages = {
             "address_line1": {"required": "Enter the street address of your organization."},
             "city": {"required": "Enter the city where your organization is located."},
-            "state_territory": {
-                "required": "Select the state, territory, or military post where your organization is located."
-            },
             "zipcode": {"required": "Enter a 5-digit or 9-digit zip code, like 12345 or 12345-6789."},
         }
         widgets = {

From 1ca52b49bbe719f4c4b1cf101df1f20ce038f06e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 14 Jan 2025 19:23:29 -0500
Subject: [PATCH 21/60] additional edits to uswds and to combobox.js to
 accomodate for empty values in options

---
 src/registrar/assets/js/uswds-edited.js       | 53 +++++++++++++------
 .../assets/src/js/getgov/combobox.js          |  2 +-
 2 files changed, 37 insertions(+), 18 deletions(-)

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index b597f2d2b..590033a87 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -1037,7 +1037,7 @@ const noop = () => {};
  * @param {string} value The new value of the element
  */
 const changeElementValue = function (el) {
-  let value = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : "";
+    let value = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : "";
   const elementToChange = el;
   elementToChange.value = value;
   const event = new CustomEvent("change", {
@@ -1167,13 +1167,21 @@ const enhanceComboBox = _comboBoxEl => {
       placeholder
     });
   }
-  if (defaultValue) {
-    for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-      const optionEl = selectEl.options[i];
-      if (optionEl.value === defaultValue) {
-        selectedOption = optionEl;
-        break;
-      }
+  // DOTGOV - allowing for defaultValue to be empty
+  //if (defaultValue) {
+  //  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+  //    const optionEl = selectEl.options[i];
+  //    if (optionEl.value === defaultValue) {
+  //      selectedOption = optionEl;
+  //      break;
+  //    }
+  //  }
+  //}
+  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+    const optionEl = selectEl.options[i];
+    if ((optionEl.value === defaultValue) || (!optionEl.value && !defaultValue)) {
+      selectedOption = optionEl;
+      break;
     }
   }
 
@@ -1500,16 +1508,27 @@ const resetSelection = el => {
   } = getComboBoxContext(el);
   const selectValue = selectEl.value;
   const inputValue = (inputEl.value || "").toLowerCase();
-  if (selectValue) {
-    for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-      const optionEl = selectEl.options[i];
-      if (optionEl.value === selectValue) {
-        if (inputValue !== optionEl.text) {
-          changeElementValue(inputEl, optionEl.text);
-        }
-        comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
-        return;
+  // DOTGOV - allow for option value to be empty string
+  //if (selectValue) {
+  //  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+  //    const optionEl = selectEl.options[i];
+  //    if (optionEl.value === selectValue) {
+  //      if (inputValue !== optionEl.text) {
+  //        changeElementValue(inputEl, optionEl.text);
+  //      }
+  //      comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
+  //      return;
+  //    }
+  //  }
+  //}
+  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+    const optionEl = selectEl.options[i];
+    if ((!selectValue && !optionEl.value) || optionEl.value === selectValue) {
+      if (inputValue !== optionEl.text) {
+        changeElementValue(inputEl, optionEl.text);
       }
+      comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
+      return;
     }
   }
   if (inputValue) {
diff --git a/src/registrar/assets/src/js/getgov/combobox.js b/src/registrar/assets/src/js/getgov/combobox.js
index 95c5ff7e8..e0ecc92ad 100644
--- a/src/registrar/assets/src/js/getgov/combobox.js
+++ b/src/registrar/assets/src/js/getgov/combobox.js
@@ -48,7 +48,7 @@ export function loadInitialValuesForComboBoxes() {
 
       // Change the default input behaviour - have it reset to the data default instead
       clearInputButton.addEventListener("click", (e) => {
-        if (overrideDefaultClearButton && initialValue) {
+        if (overrideDefaultClearButton) {
           e.preventDefault();
           e.stopPropagation();
           input.click();

From 24e0243df3004f91ca2f99722ff90f1385ee4180 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 14 Jan 2025 20:03:58 -0500
Subject: [PATCH 22/60] fixed bug with portfolio requesting entity

---
 src/registrar/assets/src/js/getgov/requesting-entity.js | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov/requesting-entity.js b/src/registrar/assets/src/js/getgov/requesting-entity.js
index 3bcdcd35c..e784419b4 100644
--- a/src/registrar/assets/src/js/getgov/requesting-entity.js
+++ b/src/registrar/assets/src/js/getgov/requesting-entity.js
@@ -9,15 +9,16 @@ export function handleRequestingEntityFieldset() {
     const formPrefix = "portfolio_requesting_entity";
     const radioFieldset = document.getElementById(`id_${formPrefix}-requesting_entity_is_suborganization__fieldset`);
     const radios = radioFieldset?.querySelectorAll(`input[name="${formPrefix}-requesting_entity_is_suborganization"]`);
-    const select = document.getElementById(`id_${formPrefix}-sub_organization`);
-    const selectParent = select?.parentElement;
+    const input = document.getElementById(`id_${formPrefix}-sub_organization`);
+    const inputGrandParent = input?.parentElement?.parentElement;
+    const select = input?.previousElementSibling;
     const suborgContainer = document.getElementById("suborganization-container");
     const suborgDetailsContainer = document.getElementById("suborganization-container__details");
     const suborgAddtlInstruction = document.getElementById("suborganization-addtl-instruction");
     const subOrgCreateNewOption = document.getElementById("option-to-add-suborg")?.value;
     // Make sure all crucial page elements exist before proceeding.
     // This more or less ensures that we are on the Requesting Entity page, and not elsewhere.
-    if (!radios || !select || !selectParent || !suborgContainer || !suborgDetailsContainer) return;
+    if (!radios || !input || !select || !inputGrandParent || !suborgContainer || !suborgDetailsContainer) return;
 
     // requestingSuborganization: This just broadly determines if they're requesting a suborg at all
     // requestingNewSuborganization: This variable determines if the user is trying to *create* a new suborganization or not.
@@ -28,7 +29,7 @@ export function handleRequestingEntityFieldset() {
         if (radio != null) requestingSuborganization = radio?.checked && radio.value === "True";
         requestingSuborganization ? showElement(suborgContainer) : hideElement(suborgContainer);
         if (select.options.length == 2) { // --Select-- and other are the only options
-            hideElement(selectParent); // Hide the select drop down and indicate requesting new suborg
+            hideElement(inputGrandParent); // Hide the combo box and indicate requesting new suborg
             hideElement(suborgAddtlInstruction); // Hide additional instruction related to the list
             requestingNewSuborganization.value = "True";
         } else {

From fabeddaa619a23192c517f831fd80a72c84e066f Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 14 Jan 2025 20:56:45 -0500
Subject: [PATCH 23/60] modified combobox to handle error class

---
 src/registrar/assets/js/uswds-edited.js | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index 590033a87..60502050f 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -31,6 +31,7 @@
  *  - fixed bug in createHeaderButton which added newlines to header button tooltips
  *  - modified combobox to allow for blank values in list
  *  - modified aria label for X button in combobox to reflect modified behavior of button
+ *  - modified combobox to handle error class
  */
 
 if ("document" in window.self) {
@@ -1223,6 +1224,11 @@ const enhanceComboBox = _comboBoxEl => {
   input.setAttribute("class", INPUT_CLASS);
   input.setAttribute("type", "text");
   input.setAttribute("role", "combobox");
+  // DOTGOV - handle error class for combobox
+  // Check if 'usa-input--error' exists in selectEl and add it to input if true
+  if (selectEl.classList.contains('usa-input--error')) {
+    input.classList.add('usa-input--error');
+  }
   additionalAttributes.forEach(attr => Object.keys(attr).forEach(key => {
     const value = Sanitizer.escapeHTML`${attr[key]}`;
     input.setAttribute(key, value);

From 74ef30b52d222171daa01c1681bdc42c797ace92 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 15 Jan 2025 12:44:47 -0500
Subject: [PATCH 24/60] fixed the 'other' problem in requesting entity form

---
 .../assets/src/js/getgov/requesting-entity.js      |  5 -----
 src/registrar/forms/domain_request_wizard.py       | 14 ++++++++++----
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov/requesting-entity.js b/src/registrar/assets/src/js/getgov/requesting-entity.js
index e784419b4..5f3be8c79 100644
--- a/src/registrar/assets/src/js/getgov/requesting-entity.js
+++ b/src/registrar/assets/src/js/getgov/requesting-entity.js
@@ -38,11 +38,6 @@ export function handleRequestingEntityFieldset() {
         requestingNewSuborganization.value === "True" ? showElement(suborgDetailsContainer) : hideElement(suborgDetailsContainer);
     }
 
-    // Add fake "other" option to sub_organization select
-    if (select && !Array.from(select.options).some(option => option.value === "other")) {
-        select.add(new Option(subOrgCreateNewOption, "other"));
-    }
-
     if (requestingNewSuborganization.value === "True") {
         select.value = "other";
     }
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 2365d323d..636a41760 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -63,13 +63,19 @@ class RequestingEntityForm(RegistrarForm):
     )
 
     def __init__(self, *args, **kwargs):
-        """Override of init to add the suborganization queryset"""
+        """Override of init to add the suborganization queryset and 'other' option"""
         super().__init__(*args, **kwargs)
 
         if self.domain_request.portfolio:
-            self.fields["sub_organization"].queryset = Suborganization.objects.filter(
-                portfolio=self.domain_request.portfolio
-            )
+            # Fetch the queryset for the portfolio
+            queryset = Suborganization.objects.filter(portfolio=self.domain_request.portfolio)
+            # set the queryset appropriately so that post can validate against queryset
+            self.fields["sub_organization"].queryset = queryset
+
+            # Modify the choices to include "other" so that form can display options properly
+            self.fields["sub_organization"].choices = [("", "--Select--")] + [
+                (obj.id, str(obj)) for obj in queryset
+            ] + [("other", "Other (enter your suborganization manually)")]
 
     def clean_sub_organization(self):
         """On suborganization clean, set the suborganization value to None if the user is requesting

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 25/60] 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 26/60] 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 27/60] 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 28/60] 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 29/60] 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 30/60] 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 31/60] 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 32/60] 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 33/60] 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 34/60] 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 35/60] 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 36/60] 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 6a49f9e373a6db7f99a50f402cb4938ab6c7597a Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 16 Jan 2025 14:13:51 -0500
Subject: [PATCH 37/60] requesting entity working - code still wip

---
 src/registrar/assets/js/uswds-edited.js      |  71 +++------
 src/registrar/assets/src/js/getgov/main.js   |   2 +-
 src/registrar/forms/domain_request_wizard.py | 150 ++++++++++++++++---
 src/registrar/views/domain_request.py        |  10 +-
 4 files changed, 163 insertions(+), 70 deletions(-)

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index 60502050f..9e3922c9c 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -1038,7 +1038,7 @@ const noop = () => {};
  * @param {string} value The new value of the element
  */
 const changeElementValue = function (el) {
-    let value = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : "";
+  let value = arguments.length > 1 && arguments[1] !== undefined ? arguments[1] : "";
   const elementToChange = el;
   elementToChange.value = value;
   const event = new CustomEvent("change", {
@@ -1168,22 +1168,14 @@ const enhanceComboBox = _comboBoxEl => {
       placeholder
     });
   }
-  // DOTGOV - allowing for defaultValue to be empty
-  //if (defaultValue) {
-  //  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-  //    const optionEl = selectEl.options[i];
-  //    if (optionEl.value === defaultValue) {
-  //      selectedOption = optionEl;
-  //      break;
-  //    }
-  //  }
-  //}
-  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-    const optionEl = selectEl.options[i];
-    if ((optionEl.value === defaultValue) || (!optionEl.value && !defaultValue)) {
-      selectedOption = optionEl;
-      break;
-    }
+  if (defaultValue) {
+   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+     const optionEl = selectEl.options[i];
+     if (optionEl.value === defaultValue) {
+       selectedOption = optionEl;
+       break;
+     }
+   }
   }
 
   /**
@@ -1234,11 +1226,9 @@ const enhanceComboBox = _comboBoxEl => {
     input.setAttribute(key, value);
   }));
   comboBoxEl.insertAdjacentElement("beforeend", input);
-  // DOTGOV - modified the aria-label of the clear input button to Reset selection to reflect changed button behavior
-  // <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Clear the select contents">&nbsp;</button>
   comboBoxEl.insertAdjacentHTML("beforeend", Sanitizer.escapeHTML`
     <span class="${CLEAR_INPUT_BUTTON_WRAPPER_CLASS}" tabindex="-1">
-        <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Reset selection">&nbsp;</button>
+        <button type="button" class="${CLEAR_INPUT_BUTTON_CLASS}" aria-label="Clear the select contents">&nbsp;</button>
       </span>
       <span class="${INPUT_BUTTON_SEPARATOR_CLASS}">&nbsp;</span>
       <span class="${TOGGLE_LIST_BUTTON_WRAPPER_CLASS}" tabindex="-1">
@@ -1374,12 +1364,8 @@ const displayList = el => {
   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
     const optionEl = selectEl.options[i];
     const optionId = `${listOptionBaseId}${options.length}`;
-    // DOTGOV: modified combobox to allow for options with blank value
-    //if (optionEl.value && (disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
-    if ((disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
-      // DOTGOV: modified combobox to allow blank option value selections to be considered selected
-      //if (selectEl.value && optionEl.value === selectEl.value) {
-      if (selectEl.value && optionEl.value === selectEl.value || (!selectEl.value && !optionEl.value)) {
+    if (optionEl.value && (disableFiltering || isPristine || !inputValue || regex.test(optionEl.text))) {
+      if (selectEl.value && optionEl.value === selectEl.value) {
         selectedItemId = optionId;
       }
       if (disableFiltering && !firstFoundId && regex.test(optionEl.text)) {
@@ -1514,28 +1500,17 @@ const resetSelection = el => {
   } = getComboBoxContext(el);
   const selectValue = selectEl.value;
   const inputValue = (inputEl.value || "").toLowerCase();
-  // DOTGOV - allow for option value to be empty string
-  //if (selectValue) {
-  //  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-  //    const optionEl = selectEl.options[i];
-  //    if (optionEl.value === selectValue) {
-  //      if (inputValue !== optionEl.text) {
-  //        changeElementValue(inputEl, optionEl.text);
-  //      }
-  //      comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
-  //      return;
-  //    }
-  //  }
-  //}
-  for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-    const optionEl = selectEl.options[i];
-    if ((!selectValue && !optionEl.value) || optionEl.value === selectValue) {
-      if (inputValue !== optionEl.text) {
-        changeElementValue(inputEl, optionEl.text);
-      }
-      comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
-      return;
-    }
+  if (selectValue) {
+   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+     const optionEl = selectEl.options[i];
+     if (optionEl.value === selectValue) {
+       if (inputValue !== optionEl.text) {
+         changeElementValue(inputEl, optionEl.text);
+       }
+       comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
+       return;
+     }
+   }
   }
   if (inputValue) {
     changeElementValue(inputEl);
diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js
index 6ff402aa4..9f3156cf7 100644
--- a/src/registrar/assets/src/js/getgov/main.js
+++ b/src/registrar/assets/src/js/getgov/main.js
@@ -31,7 +31,7 @@ initializeUrbanizationToggle();
 userProfileListener();
 finishUserSetupListener();
 
-loadInitialValuesForComboBoxes();
+//loadInitialValuesForComboBoxes();
 
 handleRequestingEntityFieldset();
 
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 636a41760..e40355d61 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -77,6 +77,20 @@ class RequestingEntityForm(RegistrarForm):
                 (obj.id, str(obj)) for obj in queryset
             ] + [("other", "Other (enter your suborganization manually)")]
 
+    @classmethod
+    def from_database(cls, obj: DomainRequest | None):
+        """Returns a dict of form field values gotten from `obj`.
+        Overrides RegistrarForm method in order to set sub_organization to 'other'
+        on GETs of the RequestingEntityForm."""
+        if obj is None:
+            return {}
+        # get the domain request as a dict, per usual method
+        domain_request_dict = {name: getattr(obj, name) for name in cls.declared_fields.keys()}  # type: ignore
+        # set sub_organization to 'other' if is_requesting_new_suborganization is True
+        if obj.is_requesting_new_suborganization():
+            domain_request_dict["sub_organization"] = "other"
+        return domain_request_dict
+
     def clean_sub_organization(self):
         """On suborganization clean, set the suborganization value to None if the user is requesting
         a custom suborganization (as it doesn't exist yet)"""
@@ -102,42 +116,132 @@ class RequestingEntityForm(RegistrarForm):
             )
         return name
 
+    # def full_clean(self):
+    #     """Validation logic to remove the custom suborganization value before clean is triggered.
+    #     Without this override, the form will throw an 'invalid option' error."""
+    #     # Remove the custom other field before cleaning
+    #     data = self.data.copy() if self.data else None
+
+    #     # Remove the 'other' value from suborganization if it exists.
+    #     # This is a special value that tracks if the user is requesting a new suborg.
+    #     suborganization = self.data.get("portfolio_requesting_entity-sub_organization")
+    #     if suborganization and "other" in suborganization:
+    #         data["portfolio_requesting_entity-sub_organization"] = ""
+
+    #     # Set the modified data back to the form
+    #     self.data = data
+
+    #     # Call the parent's full_clean method
+    #     super().full_clean()
+
     def full_clean(self):
-        """Validation logic to remove the custom suborganization value before clean is triggered.
+        """Validation logic to temporarily remove the custom suborganization value before clean is triggered.
         Without this override, the form will throw an 'invalid option' error."""
-        # Remove the custom other field before cleaning
-        data = self.data.copy() if self.data else None
+        logger.debug("full_clean")
+        # Ensure self.data is not None before proceeding
+        if self.data:
+            # handle case where form has been submitted
+            logger.debug("form was submitted")
+            # Create a copy of the data for manipulation
+            data = self.data.copy()
 
-        # Remove the 'other' value from suborganization if it exists.
-        # This is a special value that tracks if the user is requesting a new suborg.
-        suborganization = self.data.get("portfolio_requesting_entity-sub_organization")
-        if suborganization and "other" in suborganization:
-            data["portfolio_requesting_entity-sub_organization"] = ""
+            # Retrieve sub_organization
+            suborganization = data.get("portfolio_requesting_entity-sub_organization")
 
-        # Set the modified data back to the form
-        self.data = data
+            logger.debug(f"suborganization submitted as {suborganization}")
+            # # Determine if "other" should be stored in _original_suborganization
+            # if not suborganization:
+            #     logger.debug("suborg stored as other")
+            #     self._original_suborganization = "other"
+            # else:
+            self._original_suborganization = suborganization
+
+            # If the original value was "other", clear it for validation
+            if self._original_suborganization == "other":
+                data["portfolio_requesting_entity-sub_organization"] = ""
+
+            # Set the modified data back to the form
+            self.data = data
+        else:
+            # handle case of a GET
+            suborganization = None
+            if self.initial and "sub_organization" in self.initial:
+                print("suborg in self.initial")
+                suborganization = self.initial["sub_organization"]
+            print(self.initial["sub_organization"])
+            print(suborganization)
+            # Check if is_requesting_new_suborganization is True
+            is_requesting_new_suborganization = False
+            if self.initial and "is_requesting_new_suborganization" in self.initial:
+                # Call the method if it exists
+                print(self.initial["is_requesting_new_suborganization"]())
+                is_requesting_new_suborganization = self.initial["is_requesting_new_suborganization"]()
+
+            # Determine if "other" should be set
+            if is_requesting_new_suborganization and suborganization is None:
+                print("presetting to other")
+                self._original_suborganization = "other"
+            else:
+                self._original_suborganization = suborganization
+            print("self.data does not exist")
+            print(self.initial)
+            # # Handle the initial GET request case
+            # self._original_suborganization = None
 
         # Call the parent's full_clean method
         super().full_clean()
 
+        # Restore "other" if there are errors
+        if self.errors:
+            logger.debug(f"errors detected: {self.errors}; resetting original_sub_organization")
+            self.data["portfolio_requesting_entity-sub_organization"] = self._original_suborganization
+
+
+    # def clean(self):
+    #     """Custom clean implementation to handle our desired logic flow for suborganization.
+    #     Given that these fields often rely on eachother, we need to do this in the parent function."""
+    #     cleaned_data = super().clean()
+
+    #     # Do some custom error validation if the requesting entity is a suborg.
+    #     # Otherwise, just validate as normal.
+    #     suborganization = self.cleaned_data.get("sub_organization")
+    #     is_requesting_new_suborganization = self.cleaned_data.get("is_requesting_new_suborganization")
+
+    #     # Get the value of the yes/no checkbox from RequestingEntityYesNoForm.
+    #     # Since self.data stores this as a string, we need to convert "True" => True.
+    #     requesting_entity_is_suborganization = self.data.get(
+    #         "portfolio_requesting_entity-requesting_entity_is_suborganization"
+    #     )
+    #     if requesting_entity_is_suborganization == "True":
+    #         if is_requesting_new_suborganization:
+    #             # Validate custom suborganization fields
+    #             if not cleaned_data.get("requested_suborganization") and "requested_suborganization" not in self.errors:
+    #                 self.add_error("requested_suborganization", "Enter the name of your suborganization.")
+    #             if not cleaned_data.get("suborganization_city"):
+    #                 self.add_error("suborganization_city", "Enter the city where your suborganization is located.")
+    #             if not cleaned_data.get("suborganization_state_territory"):
+    #                 self.add_error(
+    #                     "suborganization_state_territory",
+    #                     "Select the state, territory, or military post where your suborganization is located.",
+    #                 )
+    #         elif not suborganization:
+    #             self.add_error("sub_organization", "Suborganization is required.")
+
+    #     return cleaned_data
+
     def clean(self):
-        """Custom clean implementation to handle our desired logic flow for suborganization.
-        Given that these fields often rely on eachother, we need to do this in the parent function."""
+        """Custom clean implementation to handle our desired logic flow for suborganization."""
         cleaned_data = super().clean()
 
-        # Do some custom error validation if the requesting entity is a suborg.
-        # Otherwise, just validate as normal.
-        suborganization = self.cleaned_data.get("sub_organization")
-        is_requesting_new_suborganization = self.cleaned_data.get("is_requesting_new_suborganization")
-
-        # Get the value of the yes/no checkbox from RequestingEntityYesNoForm.
-        # Since self.data stores this as a string, we need to convert "True" => True.
+        # Get the cleaned data
+        suborganization = cleaned_data.get("sub_organization")
+        is_requesting_new_suborganization = cleaned_data.get("is_requesting_new_suborganization")
         requesting_entity_is_suborganization = self.data.get(
             "portfolio_requesting_entity-requesting_entity_is_suborganization"
         )
+
         if requesting_entity_is_suborganization == "True":
             if is_requesting_new_suborganization:
-                # Validate custom suborganization fields
                 if not cleaned_data.get("requested_suborganization") and "requested_suborganization" not in self.errors:
                     self.add_error("requested_suborganization", "Enter the name of your suborganization.")
                 if not cleaned_data.get("suborganization_city"):
@@ -150,6 +254,12 @@ class RequestingEntityForm(RegistrarForm):
             elif not suborganization:
                 self.add_error("sub_organization", "Suborganization is required.")
 
+        # If there are errors, restore the "other" value for rendering
+        if self.errors and getattr(self, "_original_suborganization", None) == "other":
+            self.cleaned_data["sub_organization"] = self._original_suborganization
+        elif not self.data and getattr(self, "_original_suborganization", None) == "other":
+            self.cleaned_data["sub_organization"] = self._original_suborganization
+
         return cleaned_data
 
 
diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index 9754b0d0c..e0225aab3 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -368,7 +368,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView):
         and from the database if `use_db` is True (provided that record exists).
         An empty form will be provided if neither of those are true.
         """
-
+        logger.debug(f"get_forms({step},{use_post},{use_db},{files})")
         kwargs = {
             "files": files,
             "prefix": self.steps.current,
@@ -385,6 +385,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView):
 
         for form in forms:
             data = form.from_database(self.domain_request) if self.has_pk() else None
+            logger.debug(data)
             if use_post:
                 instantiated.append(form(self.request.POST, **kwargs))
             elif use_db:
@@ -562,6 +563,13 @@ class RequestingEntity(DomainRequestWizard):
     template_name = "domain_request_requesting_entity.html"
     forms = [forms.RequestingEntityYesNoForm, forms.RequestingEntityForm]
 
+    #for debugging:
+    def get(self, request, *args, **kwargs):
+        """This method handles GET requests."""
+        logger.debug("in get")
+
+        return super().get(request, *args, **kwargs)
+    
     def save(self, forms: list):
         """Override of save to clear or associate certain suborganization data
         depending on what the user wishes to do. For instance, we want to add a suborganization

From 088140a37d6dd96f3e98fa896fd5f15b02c31b1e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 16 Jan 2025 14:24:43 -0500
Subject: [PATCH 38/60] clean up of requesting entity

---
 src/registrar/forms/domain_request_wizard.py | 72 +-------------------
 src/registrar/views/domain_request.py        |  9 ---
 2 files changed, 2 insertions(+), 79 deletions(-)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index e40355d61..e452c01b1 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -116,44 +116,18 @@ class RequestingEntityForm(RegistrarForm):
             )
         return name
 
-    # def full_clean(self):
-    #     """Validation logic to remove the custom suborganization value before clean is triggered.
-    #     Without this override, the form will throw an 'invalid option' error."""
-    #     # Remove the custom other field before cleaning
-    #     data = self.data.copy() if self.data else None
-
-    #     # Remove the 'other' value from suborganization if it exists.
-    #     # This is a special value that tracks if the user is requesting a new suborg.
-    #     suborganization = self.data.get("portfolio_requesting_entity-sub_organization")
-    #     if suborganization and "other" in suborganization:
-    #         data["portfolio_requesting_entity-sub_organization"] = ""
-
-    #     # Set the modified data back to the form
-    #     self.data = data
-
-    #     # Call the parent's full_clean method
-    #     super().full_clean()
 
     def full_clean(self):
         """Validation logic to temporarily remove the custom suborganization value before clean is triggered.
         Without this override, the form will throw an 'invalid option' error."""
-        logger.debug("full_clean")
         # Ensure self.data is not None before proceeding
         if self.data:
             # handle case where form has been submitted
-            logger.debug("form was submitted")
             # Create a copy of the data for manipulation
             data = self.data.copy()
 
-            # Retrieve sub_organization
+            # Retrieve sub_organization and store in _original_suborganization
             suborganization = data.get("portfolio_requesting_entity-sub_organization")
-
-            logger.debug(f"suborganization submitted as {suborganization}")
-            # # Determine if "other" should be stored in _original_suborganization
-            # if not suborganization:
-            #     logger.debug("suborg stored as other")
-            #     self._original_suborganization = "other"
-            # else:
             self._original_suborganization = suborganization
 
             # If the original value was "other", clear it for validation
@@ -166,69 +140,27 @@ class RequestingEntityForm(RegistrarForm):
             # handle case of a GET
             suborganization = None
             if self.initial and "sub_organization" in self.initial:
-                print("suborg in self.initial")
                 suborganization = self.initial["sub_organization"]
-            print(self.initial["sub_organization"])
-            print(suborganization)
+
             # Check if is_requesting_new_suborganization is True
             is_requesting_new_suborganization = False
             if self.initial and "is_requesting_new_suborganization" in self.initial:
                 # Call the method if it exists
-                print(self.initial["is_requesting_new_suborganization"]())
                 is_requesting_new_suborganization = self.initial["is_requesting_new_suborganization"]()
 
             # Determine if "other" should be set
             if is_requesting_new_suborganization and suborganization is None:
-                print("presetting to other")
                 self._original_suborganization = "other"
             else:
                 self._original_suborganization = suborganization
-            print("self.data does not exist")
-            print(self.initial)
-            # # Handle the initial GET request case
-            # self._original_suborganization = None
 
         # Call the parent's full_clean method
         super().full_clean()
 
         # Restore "other" if there are errors
         if self.errors:
-            logger.debug(f"errors detected: {self.errors}; resetting original_sub_organization")
             self.data["portfolio_requesting_entity-sub_organization"] = self._original_suborganization
 
-
-    # def clean(self):
-    #     """Custom clean implementation to handle our desired logic flow for suborganization.
-    #     Given that these fields often rely on eachother, we need to do this in the parent function."""
-    #     cleaned_data = super().clean()
-
-    #     # Do some custom error validation if the requesting entity is a suborg.
-    #     # Otherwise, just validate as normal.
-    #     suborganization = self.cleaned_data.get("sub_organization")
-    #     is_requesting_new_suborganization = self.cleaned_data.get("is_requesting_new_suborganization")
-
-    #     # Get the value of the yes/no checkbox from RequestingEntityYesNoForm.
-    #     # Since self.data stores this as a string, we need to convert "True" => True.
-    #     requesting_entity_is_suborganization = self.data.get(
-    #         "portfolio_requesting_entity-requesting_entity_is_suborganization"
-    #     )
-    #     if requesting_entity_is_suborganization == "True":
-    #         if is_requesting_new_suborganization:
-    #             # Validate custom suborganization fields
-    #             if not cleaned_data.get("requested_suborganization") and "requested_suborganization" not in self.errors:
-    #                 self.add_error("requested_suborganization", "Enter the name of your suborganization.")
-    #             if not cleaned_data.get("suborganization_city"):
-    #                 self.add_error("suborganization_city", "Enter the city where your suborganization is located.")
-    #             if not cleaned_data.get("suborganization_state_territory"):
-    #                 self.add_error(
-    #                     "suborganization_state_territory",
-    #                     "Select the state, territory, or military post where your suborganization is located.",
-    #                 )
-    #         elif not suborganization:
-    #             self.add_error("sub_organization", "Suborganization is required.")
-
-    #     return cleaned_data
-
     def clean(self):
         """Custom clean implementation to handle our desired logic flow for suborganization."""
         cleaned_data = super().clean()
diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index e0225aab3..5fccfc2f2 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -368,7 +368,6 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView):
         and from the database if `use_db` is True (provided that record exists).
         An empty form will be provided if neither of those are true.
         """
-        logger.debug(f"get_forms({step},{use_post},{use_db},{files})")
         kwargs = {
             "files": files,
             "prefix": self.steps.current,
@@ -385,7 +384,6 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView):
 
         for form in forms:
             data = form.from_database(self.domain_request) if self.has_pk() else None
-            logger.debug(data)
             if use_post:
                 instantiated.append(form(self.request.POST, **kwargs))
             elif use_db:
@@ -562,13 +560,6 @@ class PortfolioDomainRequestWizard(DomainRequestWizard):
 class RequestingEntity(DomainRequestWizard):
     template_name = "domain_request_requesting_entity.html"
     forms = [forms.RequestingEntityYesNoForm, forms.RequestingEntityForm]
-
-    #for debugging:
-    def get(self, request, *args, **kwargs):
-        """This method handles GET requests."""
-        logger.debug("in get")
-
-        return super().get(request, *args, **kwargs)
     
     def save(self, forms: list):
         """Override of save to clear or associate certain suborganization data

From 4649198867a48408be1ca09452ecbdc62988811a Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 16 Jan 2025 12:53:16 -0700
Subject: [PATCH 39/60] Update domain.py

---
 src/registrar/models/domain.py | 52 ++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 5e186efff..a388196ca 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1665,28 +1665,6 @@ class Domain(TimeStampedModel, DomainHelper):
                 )
             return err.code
 
-    def _fetch_contacts(self, contact_data):
-        """Fetch contact info."""
-        choices = PublicContact.ContactTypeChoices
-        # We expect that all these fields get populated,
-        # so we can create these early, rather than waiting.
-        contacts_dict = {
-            choices.ADMINISTRATIVE: None,
-            choices.SECURITY: None,
-            choices.TECHNICAL: None,
-        }
-        for domainContact in contact_data:
-            req = commands.InfoContact(id=domainContact.contact)
-            data = registry.send(req, cleaned=True).res_data[0]
-
-            # Map the object we recieved from EPP to a PublicContact
-            mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type)
-
-            # Find/create it in the DB
-            in_db = self._get_or_create_public_contact(mapped_object)
-            contacts_dict[in_db.contact_type] = in_db.registry_id
-        return contacts_dict
-
     def _get_or_create_contact(self, contact: PublicContact):
         """Try to fetch info about a contact. Create it if it does not exist."""
         logger.info("_get_or_create_contact() -> Fetching contact info")
@@ -1869,8 +1847,8 @@ class Domain(TimeStampedModel, DomainHelper):
         missingSecurity = True
         missingTech = True
 
-        if len(cleaned.get("_contacts")) < 3:
-            for contact in cleaned.get("_contacts"):
+        if len(cleaned.get("contacts")) < 3:
+            for contact in cleaned.get("contacts"):
                 if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE:
                     missingAdmin = False
                 if contact.type == PublicContact.ContactTypeChoices.SECURITY:
@@ -1890,7 +1868,7 @@ class Domain(TimeStampedModel, DomainHelper):
                 technical_contact.save()
             
             logger.info(
-                "_add_missing_contacts_if_unknown => " 
+                "_add_missing_contacts_if_unknown => In function. Values are " 
                 f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}"
             )
 
@@ -2070,6 +2048,30 @@ class Domain(TimeStampedModel, DomainHelper):
         if contacts and isinstance(contacts, list) and len(contacts) > 0:
             cleaned_contacts = self._fetch_contacts(contacts)
         return cleaned_contacts
+    
+    def _fetch_contacts(self, contact_data):
+        """Fetch contact info."""
+        choices = PublicContact.ContactTypeChoices
+        # We expect that all these fields get populated,
+        # so we can create these early, rather than waiting.
+        contacts_dict = {
+            choices.ADMINISTRATIVE: None,
+            choices.SECURITY: None,
+            choices.TECHNICAL: None,
+        }
+        for domainContact in contact_data:
+            req = commands.InfoContact(id=domainContact.contact)
+            data = registry.send(req, cleaned=True).res_data[0]
+            logger.info(f"_fetch_contacts => this is the data: {data}")
+
+            # Map the object we recieved from EPP to a PublicContact
+            mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type)
+            logger.info(f"_fetch_contacts => mapped_object: {mapped_object}")
+
+            # Find/create it in the DB
+            in_db = self._get_or_create_public_contact(mapped_object)
+            contacts_dict[in_db.contact_type] = in_db.registry_id
+        return contacts_dict
 
     def _get_hosts(self, hosts):
         cleaned_hosts = []

From 511e9610e27b014f7a505f9c2f6c4c441823d615 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 16 Jan 2025 12:55:50 -0700
Subject: [PATCH 40/60] Update domain.py

---
 src/registrar/models/domain.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index a388196ca..ad549879b 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1847,8 +1847,10 @@ class Domain(TimeStampedModel, DomainHelper):
         missingSecurity = True
         missingTech = True
 
-        if len(cleaned.get("contacts")) < 3:
-            for contact in cleaned.get("contacts"):
+        # Potential collision - mismatch between _contacts and contacts?
+        # But the ID wouldnt match in this case because the default is being grabbed?
+        if len(cleaned.get("_contacts")) < 3:
+            for contact in cleaned.get("_contacts"):
                 if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE:
                     missingAdmin = False
                 if contact.type == PublicContact.ContactTypeChoices.SECURITY:

From f9fa8772e585fc30d25d439532b3a1ea8b0a5ed5 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 16 Jan 2025 15:09:32 -0500
Subject: [PATCH 41/60] cleanup and lint

---
 src/registrar/assets/js/uswds-edited.js       | 36 ++++-----
 .../assets/src/js/getgov/combobox.js          | 77 -------------------
 src/registrar/assets/src/js/getgov/main.js    |  3 -
 src/registrar/forms/domain_request_wizard.py  | 15 ++--
 src/registrar/views/domain_request.py         |  2 +-
 5 files changed, 27 insertions(+), 106 deletions(-)
 delete mode 100644 src/registrar/assets/src/js/getgov/combobox.js

diff --git a/src/registrar/assets/js/uswds-edited.js b/src/registrar/assets/js/uswds-edited.js
index 9e3922c9c..9d4dd2e51 100644
--- a/src/registrar/assets/js/uswds-edited.js
+++ b/src/registrar/assets/js/uswds-edited.js
@@ -29,8 +29,6 @@
  *  - tooltip dynamic content updated to include nested element (for better sizing control)
  *  - modal exposed to window to be accessible in other js files
  *  - fixed bug in createHeaderButton which added newlines to header button tooltips
- *  - modified combobox to allow for blank values in list
- *  - modified aria label for X button in combobox to reflect modified behavior of button
  *  - modified combobox to handle error class
  */
 
@@ -1169,13 +1167,13 @@ const enhanceComboBox = _comboBoxEl => {
     });
   }
   if (defaultValue) {
-   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-     const optionEl = selectEl.options[i];
-     if (optionEl.value === defaultValue) {
-       selectedOption = optionEl;
-       break;
-     }
-   }
+    for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+      const optionEl = selectEl.options[i];
+      if (optionEl.value === defaultValue) {
+        selectedOption = optionEl;
+        break;
+      }
+    }
   }
 
   /**
@@ -1501,16 +1499,16 @@ const resetSelection = el => {
   const selectValue = selectEl.value;
   const inputValue = (inputEl.value || "").toLowerCase();
   if (selectValue) {
-   for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
-     const optionEl = selectEl.options[i];
-     if (optionEl.value === selectValue) {
-       if (inputValue !== optionEl.text) {
-         changeElementValue(inputEl, optionEl.text);
-       }
-       comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
-       return;
-     }
-   }
+    for (let i = 0, len = selectEl.options.length; i < len; i += 1) {
+      const optionEl = selectEl.options[i];
+      if (optionEl.value === selectValue) {
+        if (inputValue !== optionEl.text) {
+          changeElementValue(inputEl, optionEl.text);
+        }
+        comboBoxEl.classList.add(COMBO_BOX_PRISTINE_CLASS);
+        return;
+      }
+    }
   }
   if (inputValue) {
     changeElementValue(inputEl);
diff --git a/src/registrar/assets/src/js/getgov/combobox.js b/src/registrar/assets/src/js/getgov/combobox.js
deleted file mode 100644
index e0ecc92ad..000000000
--- a/src/registrar/assets/src/js/getgov/combobox.js
+++ /dev/null
@@ -1,77 +0,0 @@
-import { hideElement, showElement } from './helpers.js';
-
-export function loadInitialValuesForComboBoxes() {
-  var overrideDefaultClearButton = true;
-  var isTyping = false;
-
-  document.addEventListener('DOMContentLoaded', (event) => {
-    handleAllComboBoxElements();
-  });
-
-  function handleAllComboBoxElements() {
-    const comboBoxElements = document.querySelectorAll(".usa-combo-box");
-    comboBoxElements.forEach(comboBox => {
-      const input = comboBox.querySelector("input");
-      const select = comboBox.querySelector("select");
-      if (!input || !select) {
-        console.warn("No combobox element found");
-        return;
-      }
-      // Set the initial value of the combobox
-      let initialValue = select.getAttribute("data-default-value");
-      let clearInputButton = comboBox.querySelector(".usa-combo-box__clear-input");
-      if (!clearInputButton) {
-        console.warn("No clear element found");
-        return;
-      }
-
-      // Override the default clear button behavior such that it no longer clears the input,
-      // it just resets to the data-initial-value.
-      // Due to the nature of how uswds works, this is slightly hacky.
-
-      // Input event listener to detect typing
-      input.addEventListener("input", () => {
-        isTyping = true;
-      });
-
-      // Blur event listener to reset typing state
-      input.addEventListener("blur", () => {
-        isTyping = false;
-      });
-
-      // Hide the reset button when there is nothing to reset.
-      // Do this once on init, then everytime a change occurs.
-      updateClearButtonVisibility(select, initialValue, clearInputButton)
-      select.addEventListener("change", () => {
-        updateClearButtonVisibility(select, initialValue, clearInputButton)
-      });
-
-      // Change the default input behaviour - have it reset to the data default instead
-      clearInputButton.addEventListener("click", (e) => {
-        if (overrideDefaultClearButton) {
-          e.preventDefault();
-          e.stopPropagation();
-          input.click();
-          // Find the dropdown option with the desired value
-          const dropdownOptions = document.querySelectorAll(".usa-combo-box__list-option");
-          if (dropdownOptions) {
-            dropdownOptions.forEach(option => {
-                if (option.getAttribute("data-value") === initialValue) {
-                    // Simulate a click event on the dropdown option
-                    option.click();
-                }
-            });
-          }
-        }
-      });
-    });
-  }
-
-  function updateClearButtonVisibility(select, initialValue, clearInputButton) {
-    if (select.value === initialValue) {
-      hideElement(clearInputButton);
-    }else {
-      showElement(clearInputButton)
-    }
-  }
-}
diff --git a/src/registrar/assets/src/js/getgov/main.js b/src/registrar/assets/src/js/getgov/main.js
index 9f3156cf7..a077da929 100644
--- a/src/registrar/assets/src/js/getgov/main.js
+++ b/src/registrar/assets/src/js/getgov/main.js
@@ -3,7 +3,6 @@ import { initDomainValidators } from './domain-validators.js';
 import { initFormsetsForms, triggerModalOnDsDataForm, nameserversFormListener } from './formset-forms.js';
 import { initializeUrbanizationToggle } from './urbanization.js';
 import { userProfileListener, finishUserSetupListener } from './user-profile.js';
-import { loadInitialValuesForComboBoxes } from './combobox.js';
 import { handleRequestingEntityFieldset } from './requesting-entity.js';
 import { initDomainsTable } from './table-domains.js';
 import { initDomainRequestsTable } from './table-domain-requests.js';
@@ -31,8 +30,6 @@ initializeUrbanizationToggle();
 userProfileListener();
 finishUserSetupListener();
 
-//loadInitialValuesForComboBoxes();
-
 handleRequestingEntityFieldset();
 
 initDomainsTable();
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index e452c01b1..12d3b74bf 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -73,12 +73,14 @@ class RequestingEntityForm(RegistrarForm):
             self.fields["sub_organization"].queryset = queryset
 
             # Modify the choices to include "other" so that form can display options properly
-            self.fields["sub_organization"].choices = [("", "--Select--")] + [
-                (obj.id, str(obj)) for obj in queryset
-            ] + [("other", "Other (enter your suborganization manually)")]
+            self.fields["sub_organization"].choices = (
+                [("", "--Select--")]
+                + [(obj.id, str(obj)) for obj in queryset]
+                + [("other", "Other (enter your suborganization manually)")]
+            )
 
     @classmethod
-    def from_database(cls, obj: DomainRequest | None):
+    def from_database(cls, obj: DomainRequest | Contact | None):
         """Returns a dict of form field values gotten from `obj`.
         Overrides RegistrarForm method in order to set sub_organization to 'other'
         on GETs of the RequestingEntityForm."""
@@ -86,9 +88,11 @@ class RequestingEntityForm(RegistrarForm):
             return {}
         # get the domain request as a dict, per usual method
         domain_request_dict = {name: getattr(obj, name) for name in cls.declared_fields.keys()}  # type: ignore
+
         # set sub_organization to 'other' if is_requesting_new_suborganization is True
-        if obj.is_requesting_new_suborganization():
+        if isinstance(obj, DomainRequest) and obj.is_requesting_new_suborganization():
             domain_request_dict["sub_organization"] = "other"
+
         return domain_request_dict
 
     def clean_sub_organization(self):
@@ -116,7 +120,6 @@ class RequestingEntityForm(RegistrarForm):
             )
         return name
 
-
     def full_clean(self):
         """Validation logic to temporarily remove the custom suborganization value before clean is triggered.
         Without this override, the form will throw an 'invalid option' error."""
diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index 5fccfc2f2..bff3e5c00 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -560,7 +560,7 @@ class PortfolioDomainRequestWizard(DomainRequestWizard):
 class RequestingEntity(DomainRequestWizard):
     template_name = "domain_request_requesting_entity.html"
     forms = [forms.RequestingEntityYesNoForm, forms.RequestingEntityForm]
-    
+
     def save(self, forms: list):
         """Override of save to clear or associate certain suborganization data
         depending on what the user wishes to do. For instance, we want to add a suborganization

From 1ece895ac6a6c51d34f41b03a3210ea428185d1b Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 16 Jan 2025 15:15:36 -0700
Subject: [PATCH 42/60] Add test for race condition

---
 src/registrar/models/domain.py            | 29 ++++++++------
 src/registrar/tests/test_models_domain.py | 48 +++++++++++++++++++++++
 2 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index ad549879b..94ab21bde 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1939,6 +1939,8 @@ class Domain(TimeStampedModel, DomainHelper):
         Additionally, capture and cache old hosts and contacts from cache if they
         don't exist in cleaned
         """
+
+        # object reference issue between self._cache vs cleaned?
         old_cache_hosts = self._cache.get("hosts")
         old_cache_contacts = self._cache.get("contacts")
 
@@ -2111,19 +2113,20 @@ class Domain(TimeStampedModel, DomainHelper):
         # Save to DB if it doesn't exist already.
         if db_contact.count() == 0:
             # Doesn't run custom save logic, just saves to DB
-            try:
-                public_contact.save(skip_epp_save=True)
-                logger.info(f"Created a new PublicContact: {public_contact}")
-            except IntegrityError as err:
-                logger.error(
-                    "_get_or_create_public_contact() => tried to create a duplicate public contact: "
-                    f"{err}", exc_info=True
-                )
-                return PublicContact.objects.get(
-                    registry_id=public_contact.registry_id,
-                    contact_type=public_contact.contact_type,
-                    domain=self,
-                )
+            public_contact.save(skip_epp_save=True)
+            # try:
+            #     public_contact.save(skip_epp_save=True)
+            #     logger.info(f"Created a new PublicContact: {public_contact}")
+            # except IntegrityError as err:
+            #     logger.error(
+            #         "_get_or_create_public_contact() => tried to create a duplicate public contact: "
+            #         f"{err}", exc_info=True
+            #     )
+            #     return PublicContact.objects.get(
+            #         registry_id=public_contact.registry_id,
+            #         contact_type=public_contact.contact_type,
+            #         domain=self,
+            #     )
 
             # Append the item we just created
             return public_contact
diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py
index 15a88a608..56d5cf2be 100644
--- a/src/registrar/tests/test_models_domain.py
+++ b/src/registrar/tests/test_models_domain.py
@@ -348,6 +348,54 @@ class TestDomainCache(MockEppLib):
 
 class TestDomainCreation(MockEppLib):
     """Rule: An approved domain request must result in a domain"""
+    def test_get_security_email_during_unknown_state_race_condition(self):
+        """
+        Scenario: A domain is accessed for the first time
+            Given a domain in UNKNOWN state with a security contact in registry
+            When get_security_email is called during state transition
+            Then the security contact is fetched from registry
+            And only one security contact exists in database
+            And the security email matches the registry contact
+            And no duplicate contact creation is attempted
+        """
+        domain, _ = Domain.objects.get_or_create(name="defaultsecurity.gov")
+        
+        # Store original method
+        original_filter = PublicContact.objects.filter
+        
+        def mock_filter(*args, **kwargs):
+            # First call returns empty queryset to simulate contact not existing
+            result = original_filter(*args, **kwargs)
+            if kwargs.get('contact_type') == PublicContact.ContactTypeChoices.SECURITY:
+                # Create the duplicate contact after the check but before the save
+                duplicate = PublicContact(
+                    domain=domain,
+                    contact_type=PublicContact.ContactTypeChoices.SECURITY,
+                    registry_id="defaultSec",
+                    email="dotgov@cisa.dhs.gov",
+                    name="Registry Customer Service"
+                )
+                duplicate.save(skip_epp_save=True)
+            return result
+            
+        with patch.object(PublicContact.objects, 'filter', side_effect=mock_filter):
+            try:
+                security_email = domain.get_security_email()
+            except IntegrityError:
+                self.fail(
+                    "IntegrityError was raised during contact creation due to a race condition. "
+                    "This indicates that concurrent contact creation is not working in some cases. "
+                    "The error occurs when two processes try to create the same contact simultaneously. "
+                    "Expected behavior: gracefully handle duplicate creation and return existing contact."
+                )
+
+        # Verify only one contact exists
+        security_contacts = PublicContact.objects.filter(
+            domain=domain,
+            contact_type=PublicContact.ContactTypeChoices.SECURITY
+        )
+        self.assertEqual(security_contacts.count(), 1)
+        self.assertEqual(security_email, "dotgov@cisa.dhs.gov")
 
     @boto3_mocking.patching
     def test_approved_domain_request_creates_domain_locally(self):

From 4de586005e0ed88583d343dd989d1b1bc1a09a63 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 16 Jan 2025 15:40:03 -0700
Subject: [PATCH 43/60] Test part 2

---
 src/registrar/models/domain.py            | 32 +++++++++++--------
 src/registrar/tests/test_models_domain.py | 39 +++++++++++++++--------
 2 files changed, 44 insertions(+), 27 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 94ab21bde..aaafd6aa8 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -2086,11 +2086,13 @@ class Domain(TimeStampedModel, DomainHelper):
     def _get_or_create_public_contact(self, public_contact: PublicContact):
         """Tries to find a PublicContact object in our DB.
         If it can't, it'll create it. Returns PublicContact"""
+        logger.info(f"in function")
         db_contact = PublicContact.objects.filter(
             registry_id=public_contact.registry_id,
             contact_type=public_contact.contact_type,
             domain=self,
         )
+        logger.info(f"db_contact {db_contact}")
 
         # If we find duplicates, log it and delete the oldest ones.
         if db_contact.count() > 1:
@@ -2113,20 +2115,22 @@ class Domain(TimeStampedModel, DomainHelper):
         # Save to DB if it doesn't exist already.
         if db_contact.count() == 0:
             # Doesn't run custom save logic, just saves to DB
-            public_contact.save(skip_epp_save=True)
-            # try:
-            #     public_contact.save(skip_epp_save=True)
-            #     logger.info(f"Created a new PublicContact: {public_contact}")
-            # except IntegrityError as err:
-            #     logger.error(
-            #         "_get_or_create_public_contact() => tried to create a duplicate public contact: "
-            #         f"{err}", exc_info=True
-            #     )
-            #     return PublicContact.objects.get(
-            #         registry_id=public_contact.registry_id,
-            #         contact_type=public_contact.contact_type,
-            #         domain=self,
-            #     )
+            try:
+                public_contact.save(skip_epp_save=True)
+                logger.info(f"Created a new PublicContact: {public_contact}")
+            # In rare cases, _add_missing_contacts_if_unknown will cause a race condition with this function.
+            # This is because it calls .save(), which is called here.
+            # 
+            except IntegrityError as err:
+                logger.error(
+                    "_get_or_create_public_contact() => tried to create a duplicate public contact: "
+                    f"{err}", exc_info=True
+                )
+                return PublicContact.objects.get(
+                    registry_id=public_contact.registry_id,
+                    contact_type=public_contact.contact_type,
+                    domain=self,
+                )
 
             # Append the item we just created
             return public_contact
diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py
index 56d5cf2be..bb1974464 100644
--- a/src/registrar/tests/test_models_domain.py
+++ b/src/registrar/tests/test_models_domain.py
@@ -348,26 +348,30 @@ class TestDomainCache(MockEppLib):
 
 class TestDomainCreation(MockEppLib):
     """Rule: An approved domain request must result in a domain"""
-    def test_get_security_email_during_unknown_state_race_condition(self):
+
+    def test_get_security_email_race_condition(self):
         """
-        Scenario: A domain is accessed for the first time
-            Given a domain in UNKNOWN state with a security contact in registry
-            When get_security_email is called during state transition
-            Then the security contact is fetched from registry
+        Scenario: Two processes try to create the same security contact simultaneously
+            Given a domain in UNKNOWN state
+            When a race condition occurs during contact creation
+            Then no IntegrityError is raised
             And only one security contact exists in database
-            And the security email matches the registry contact
-            And no duplicate contact creation is attempted
+            And the correct security email is returned
         """
         domain, _ = Domain.objects.get_or_create(name="defaultsecurity.gov")
         
         # Store original method
         original_filter = PublicContact.objects.filter
-        
+        self.first_call = True
         def mock_filter(*args, **kwargs):
-            # First call returns empty queryset to simulate contact not existing
+            """ Simulates the race condition by creating a 
+            duplicate contact between filter check and save
+            """
             result = original_filter(*args, **kwargs)
-            if kwargs.get('contact_type') == PublicContact.ContactTypeChoices.SECURITY:
-                # Create the duplicate contact after the check but before the save
+
+            # Return empty queryset for first call. Otherwise just proceed as normal.
+            if self.first_call:
+                self.first_call = False
                 duplicate = PublicContact(
                     domain=domain,
                     contact_type=PublicContact.ContactTypeChoices.SECURITY,
@@ -376,11 +380,20 @@ class TestDomainCreation(MockEppLib):
                     name="Registry Customer Service"
                 )
                 duplicate.save(skip_epp_save=True)
+                return PublicContact.objects.none()
+
             return result
             
         with patch.object(PublicContact.objects, 'filter', side_effect=mock_filter):
             try:
-                security_email = domain.get_security_email()
+                public_contact = PublicContact(
+                    domain=domain,
+                    contact_type=PublicContact.ContactTypeChoices.SECURITY,
+                    registry_id="defaultSec",
+                    email="dotgov@cisa.dhs.gov",
+                    name="Registry Customer Service"
+                )
+                returned_public_contact = domain._get_or_create_public_contact(public_contact)
             except IntegrityError:
                 self.fail(
                     "IntegrityError was raised during contact creation due to a race condition. "
@@ -395,7 +408,7 @@ class TestDomainCreation(MockEppLib):
             contact_type=PublicContact.ContactTypeChoices.SECURITY
         )
         self.assertEqual(security_contacts.count(), 1)
-        self.assertEqual(security_email, "dotgov@cisa.dhs.gov")
+        self.assertEqual(returned_public_contact, security_contacts.get())
 
     @boto3_mocking.patching
     def test_approved_domain_request_creates_domain_locally(self):

From cfcdd8826cb799701207ebab68e8aa06f11fe889 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Thu, 16 Jan 2025 16:04:10 -0700
Subject: [PATCH 44/60] Squash bug

---
 src/registrar/models/domain.py            | 5 +++--
 src/registrar/tests/test_models_domain.py | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index aaafd6aa8..f7e76cbc4 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -2116,8 +2116,9 @@ class Domain(TimeStampedModel, DomainHelper):
         if db_contact.count() == 0:
             # Doesn't run custom save logic, just saves to DB
             try:
-                public_contact.save(skip_epp_save=True)
-                logger.info(f"Created a new PublicContact: {public_contact}")
+                with transaction.atomic():
+                    public_contact.save(skip_epp_save=True)
+                    logger.info(f"Created a new PublicContact: {public_contact}")
             # In rare cases, _add_missing_contacts_if_unknown will cause a race condition with this function.
             # This is because it calls .save(), which is called here.
             # 
diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py
index bb1974464..8fde89fd0 100644
--- a/src/registrar/tests/test_models_domain.py
+++ b/src/registrar/tests/test_models_domain.py
@@ -349,6 +349,7 @@ class TestDomainCache(MockEppLib):
 class TestDomainCreation(MockEppLib):
     """Rule: An approved domain request must result in a domain"""
 
+    @less_console_noise_decorator
     def test_get_security_email_race_condition(self):
         """
         Scenario: Two processes try to create the same security contact simultaneously

From 2ad8e0268adebcf32b0d5092374a0c012f8c18d6 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 16 Jan 2025 19:05:37 -0500
Subject: [PATCH 45/60] changes to domain request data full report and test

---
 src/registrar/tests/common.py       |  8 ++++++
 src/registrar/tests/test_reports.py | 25 +++++++++++-------
 src/registrar/utility/csv_export.py | 39 +++++++++++++++++++++++++++--
 3 files changed, 61 insertions(+), 11 deletions(-)

diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index c0083068d..2eff53d39 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -578,6 +578,10 @@ class MockDb(TestCase):
             creator=cls.custom_superuser, federal_agency=cls.federal_agency_3, organization_type="federal"
         )
 
+        cls.suborganization_1, _ = Suborganization.objects.get_or_create(
+            name="SubOrg 1", portfolio=cls.portfolio_1, city="Nashville", state_territory="TN",
+        )
+
         current_date = get_time_aware_date(datetime(2024, 4, 2))
         # Create start and end dates using timedelta
 
@@ -848,6 +852,7 @@ class MockDb(TestCase):
                 status=DomainRequest.DomainRequestStatus.IN_REVIEW,
                 name="city2.gov",
                 portfolio=cls.portfolio_1,
+                sub_organization=cls.suborganization_1,
             )
             cls.domain_request_3 = completed_domain_request(
                 status=DomainRequest.DomainRequestStatus.STARTED,
@@ -863,6 +868,9 @@ class MockDb(TestCase):
             cls.domain_request_5 = completed_domain_request(
                 status=DomainRequest.DomainRequestStatus.APPROVED,
                 name="city5.gov",
+                requested_suborganization="requested_suborg",
+                suborganization_city="SanFran",
+                suborganization_state_territory="CA",
             )
             cls.domain_request_6 = completed_domain_request(
                 status=DomainRequest.DomainRequestStatus.STARTED,
diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index b11500ea9..cabaa048d 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -729,6 +729,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             # "Submitted at",
             "Status",
             "Domain type",
+            "Portfolio",
             "Federal type",
             "Federal agency",
             "Organization name",
@@ -736,6 +737,10 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             "City",
             "State/territory",
             "Region",
+            "Suborganization",
+            "Requested suborg",
+            "Suborg city",
+            "Suborg state/territory",
             "Creator first name",
             "Creator last name",
             "Creator email",
@@ -765,27 +770,29 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
 
             expected_content = (
                 # Header
-                "Domain request,Status,Domain type,Federal type,Federal agency,Organization name,Election office,"
-                "City,State/territory,Region,Creator first name,Creator last name,Creator email,"
+                "Domain request,Status,Domain type,Portfolio,Federal type,Federal agency,Organization name,"
+                "Election office,City,State/territory,Region,Suborganization,Requested suborg,Suborg city,"
+                "Suborg state/territory,Creator first name,Creator last name,Creator email,"
                 "Creator approved domains count,Creator active requests count,Alternative domains,SO first name,"
                 "SO last name,SO email,SO title/role,Request purpose,Request additional details,Other contacts,"
                 "CISA regional representative,Current websites,Investigator\n"
                 # Content
-                "city5.gov,Approved,Federal,Executive,,Testorg,N/A,,NY,2,,,,1,0,city1.gov,Testy,Tester,testy@town.com,"
-                "Chief Tester,Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n"
-                "city2.gov,In review,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1,city1.gov,,,,,"
-                "Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n"
-                "city3.gov,Submitted,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1,"
+                "city5.gov,Approved,Federal,No,Executive,,Testorg,N/A,,NY,2,requested_suborg,SanFran,CA,,,,,1,0,"
+                "city1.gov,Testy,Tester,testy@town.com,Chief Tester,Purpose of the site,There is more,"
+                "Testy Tester testy2@town.com,,city.com,\n"
+                "city2.gov,In review,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,SubOrg 1,,,,,,,0,"
+                "1,city1.gov,,,,,Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n"
+                "city3.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1,"
                 '"cheeseville.gov, city1.gov, igorville.gov",,,,,Purpose of the site,CISA-first-name CISA-last-name | '
                 'There is more,"Meow Tester24 te2@town.com, Testy1232 Tester24 te2@town.com, '
                 'Testy Tester testy2@town.com",'
                 'test@igorville.com,"city.com, https://www.example2.com, https://www.example.com",\n'
-                "city4.gov,Submitted,City,Executive,,Testorg,Yes,,NY,2,,,,0,1,city1.gov,Testy,"
+                "city4.gov,Submitted,City,No,Executive,,Testorg,Yes,,NY,2,,,,,,,,0,1,city1.gov,Testy,"
                 "Tester,testy@town.com,"
                 "Chief Tester,Purpose of the site,CISA-first-name CISA-last-name | There is more,"
                 "Testy Tester testy2@town.com,"
                 "cisaRep@igorville.gov,city.com,\n"
-                "city6.gov,Submitted,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,NY,2,,,,0,1,city1.gov,,,,,"
+                "city6.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1,city1.gov,,,,,"
                 "Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com,"
                 "cisaRep@igorville.gov,city.com,\n"
             )
diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py
index 6e5773ebb..1bb53a7a3 100644
--- a/src/registrar/utility/csv_export.py
+++ b/src/registrar/utility/csv_export.py
@@ -1660,6 +1660,27 @@ class DomainRequestExport(BaseExport):
                 default=F("organization_name"),
                 output_field=CharField(),
             ),
+            "converted_city": Case(
+                # When portfolio is present, use its value instead
+                When(portfolio__isnull=False, then=F("portfolio__city")),
+                # Otherwise, return the natively assigned value
+                default=F("city"),
+                output_field=CharField(),
+            ),
+            "converted_state_territory": Case(
+                # When portfolio is present, use its value instead
+                When(portfolio__isnull=False, then=F("portfolio__state_territory")),
+                # Otherwise, return the natively assigned value
+                default=F("state_territory"),
+                output_field=CharField(),
+            ),
+            "converted_suborganization_name": Case(
+                # When sub_organization is present, use its name
+                When(sub_organization__isnull=False, then=F("sub_organization__name")),
+                # Otherwise, return empty string
+                default=Value(""),
+                output_field=CharField(),
+            ),
             "converted_so_email": Case(
                 # When portfolio is present, use its value instead
                 When(portfolio__isnull=False, then=F("portfolio__senior_official__email")),
@@ -1786,6 +1807,10 @@ class DomainRequestExport(BaseExport):
         status = model.get("status")
         status_display = DomainRequest.DomainRequestStatus.get_status_label(status) if status else None
 
+        # Handle the portfolio field. Display as a Yes/No
+        portfolio = model.get("portfolio")
+        portfolio_display = "Yes" if portfolio is not None else "No"
+
         # Handle the region field.
         state_territory = model.get("state_territory")
         region = get_region(state_territory) if state_territory else None
@@ -1819,6 +1844,7 @@ class DomainRequestExport(BaseExport):
             "Election office": human_readable_election_board,
             "Federal type": human_readable_federal_type,
             "Domain type": human_readable_org_type,
+            "Portfolio": portfolio_display,
             "Request additional details": additional_details,
             # Annotated fields - passed into the request dict.
             "Creator approved domains count": model.get("creator_approved_domains_count", 0),
@@ -1827,6 +1853,10 @@ class DomainRequestExport(BaseExport):
             "Other contacts": model.get("all_other_contacts"),
             "Current websites": model.get("all_current_websites"),
             # Untouched FK fields - passed into the request dict.
+            "Suborganization": model.get("converted_suborganization_name"),
+            "Requested suborg": model.get("requested_suborganization"),
+            "Suborg city": model.get("suborganization_city"),
+            "Suborg state/territory": model.get("suborganization_state_territory"),
             "Federal agency": model.get("converted_federal_agency"),
             "SO first name": model.get("converted_senior_official_first_name"),
             "SO last name": model.get("converted_senior_official_last_name"),
@@ -1838,8 +1868,8 @@ class DomainRequestExport(BaseExport):
             "Investigator": model.get("investigator__email"),
             # Untouched fields
             "Organization name": model.get("converted_organization_name"),
-            "City": model.get("city"),
-            "State/territory": model.get("state_territory"),
+            "City": model.get("converted_city"),
+            "State/territory": model.get("converted_state_territory"),
             "Request purpose": model.get("purpose"),
             "CISA regional representative": model.get("cisa_representative_email"),
             "Last submitted date": model.get("last_submitted_date"),
@@ -2006,6 +2036,7 @@ class DomainRequestDataFull(DomainRequestExport):
             "Last status update",
             "Status",
             "Domain type",
+            "Portfolio",
             "Federal type",
             "Federal agency",
             "Organization name",
@@ -2013,6 +2044,10 @@ class DomainRequestDataFull(DomainRequestExport):
             "City",
             "State/territory",
             "Region",
+            "Suborganization",
+            "Requested suborg",
+            "Suborg city",
+            "Suborg state/territory",
             "Creator first name",
             "Creator last name",
             "Creator email",

From 75665ff378c5216fc7281f0637c24db8cb956cf2 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 16 Jan 2025 19:31:11 -0500
Subject: [PATCH 46/60] fixed other tests and linted

---
 src/registrar/tests/common.py                  |  5 ++++-
 src/registrar/tests/test_management_scripts.py | 10 +++++++++-
 src/registrar/tests/test_reports.py            |  4 ++--
 3 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index 2eff53d39..bb65ef6b1 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -579,7 +579,10 @@ class MockDb(TestCase):
         )
 
         cls.suborganization_1, _ = Suborganization.objects.get_or_create(
-            name="SubOrg 1", portfolio=cls.portfolio_1, city="Nashville", state_territory="TN",
+            name="SubOrg 1",
+            portfolio=cls.portfolio_1,
+            city="Nashville",
+            state_territory="TN",
         )
 
         current_date = get_time_aware_date(datetime(2024, 4, 2))
diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py
index 655068493..536d1e760 100644
--- a/src/registrar/tests/test_management_scripts.py
+++ b/src/registrar/tests/test_management_scripts.py
@@ -2101,6 +2101,10 @@ class TestPatchSuborganizations(MockDbForIndividualTests):
         1. Fewest spaces
         2. Most leading capitals
         """
+        # Delete any other suborganizations defined in the initial test dataset
+        DomainRequest.objects.all().delete()
+        Suborganization.objects.all().delete()
+
         Suborganization.objects.create(name="Test Organization ", portfolio=self.portfolio_1)
         Suborganization.objects.create(name="test organization", portfolio=self.portfolio_1)
         Suborganization.objects.create(name="Test Organization", portfolio=self.portfolio_1)
@@ -2114,6 +2118,10 @@ class TestPatchSuborganizations(MockDbForIndividualTests):
     @less_console_noise_decorator
     def test_hardcoded_record(self):
         """Tests that our hardcoded records update as we expect them to"""
+        # Delete any other suborganizations defined in the initial test dataset
+        DomainRequest.objects.all().delete()
+        Suborganization.objects.all().delete()
+
         # Create orgs with old and new name formats
         old_name = "USDA/OC"
         new_name = "USDA, Office of Communications"
@@ -2123,7 +2131,7 @@ class TestPatchSuborganizations(MockDbForIndividualTests):
 
         self.run_patch_suborganizations()
 
-        # Verify only the new one remains
+        # Verify only the new one of the two remains
         self.assertEqual(Suborganization.objects.count(), 1)
         remaining = Suborganization.objects.first()
         self.assertEqual(remaining.name, new_name)
diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index cabaa048d..9d410e430 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -792,8 +792,8 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
                 "Chief Tester,Purpose of the site,CISA-first-name CISA-last-name | There is more,"
                 "Testy Tester testy2@town.com,"
                 "cisaRep@igorville.gov,city.com,\n"
-                "city6.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1,city1.gov,,,,,"
-                "Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com,"
+                "city6.gov,Submitted,Federal,Yes,Executive,Portfolio 1 Federal Agency,,N/A,,,2,,,,,,,,0,1,city1.gov,"
+                ",,,,Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com,"
                 "cisaRep@igorville.gov,city.com,\n"
             )
 

From a0d6e1ec3356e9765e855b9c5a0766c734cacad8 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 17 Jan 2025 10:06:40 -0700
Subject: [PATCH 47/60] cleanup

---
 src/registrar/models/domain.py            | 24 +++++--------
 src/registrar/tests/test_models_domain.py | 42 ++++++++++++-----------
 2 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index f7e76cbc4..2ace5f22d 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1847,10 +1847,9 @@ class Domain(TimeStampedModel, DomainHelper):
         missingSecurity = True
         missingTech = True
 
-        # Potential collision - mismatch between _contacts and contacts?
-        # But the ID wouldnt match in this case because the default is being grabbed?
-        if len(cleaned.get("_contacts")) < 3:
-            for contact in cleaned.get("_contacts"):
+        contacts = cleaned.get("_contacts", [])
+        if len(contacts) < 3:
+            for contact in contacts:
                 if contact.type == PublicContact.ContactTypeChoices.ADMINISTRATIVE:
                     missingAdmin = False
                 if contact.type == PublicContact.ContactTypeChoices.SECURITY:
@@ -1868,9 +1867,9 @@ class Domain(TimeStampedModel, DomainHelper):
             if missingTech:
                 technical_contact = self.get_default_technical_contact()
                 technical_contact.save()
-            
+
             logger.info(
-                "_add_missing_contacts_if_unknown => In function. Values are " 
+                "_add_missing_contacts_if_unknown => Adding contacts. Values are "
                 f"missingAdmin: {missingAdmin}, missingSecurity: {missingSecurity}, missingTech: {missingTech}"
             )
 
@@ -2052,7 +2051,7 @@ class Domain(TimeStampedModel, DomainHelper):
         if contacts and isinstance(contacts, list) and len(contacts) > 0:
             cleaned_contacts = self._fetch_contacts(contacts)
         return cleaned_contacts
-    
+
     def _fetch_contacts(self, contact_data):
         """Fetch contact info."""
         choices = PublicContact.ContactTypeChoices
@@ -2086,13 +2085,11 @@ class Domain(TimeStampedModel, DomainHelper):
     def _get_or_create_public_contact(self, public_contact: PublicContact):
         """Tries to find a PublicContact object in our DB.
         If it can't, it'll create it. Returns PublicContact"""
-        logger.info(f"in function")
         db_contact = PublicContact.objects.filter(
             registry_id=public_contact.registry_id,
             contact_type=public_contact.contact_type,
             domain=self,
         )
-        logger.info(f"db_contact {db_contact}")
 
         # If we find duplicates, log it and delete the oldest ones.
         if db_contact.count() > 1:
@@ -2119,13 +2116,10 @@ class Domain(TimeStampedModel, DomainHelper):
                 with transaction.atomic():
                     public_contact.save(skip_epp_save=True)
                     logger.info(f"Created a new PublicContact: {public_contact}")
-            # In rare cases, _add_missing_contacts_if_unknown will cause a race condition with this function.
-            # This is because it calls .save(), which is called here.
-            # 
             except IntegrityError as err:
                 logger.error(
-                    "_get_or_create_public_contact() => tried to create a duplicate public contact: "
-                    f"{err}", exc_info=True
+                    "_get_or_create_public_contact() => tried to create a duplicate public contact: " f"{err}",
+                    exc_info=True,
                 )
                 return PublicContact.objects.get(
                     registry_id=public_contact.registry_id,
@@ -2142,7 +2136,7 @@ class Domain(TimeStampedModel, DomainHelper):
         if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id:
             existing_contact.delete()
             public_contact.save()
-            logger.warning("Requested PublicContact is out of sync " "with DB.")
+            logger.warning("Requested PublicContact is out of sync with our DB.")
             return public_contact
 
         # If it already exists, we can assume that the DB instance was updated during set, so we should just use that.
diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py
index 8fde89fd0..083725a55 100644
--- a/src/registrar/tests/test_models_domain.py
+++ b/src/registrar/tests/test_models_domain.py
@@ -350,27 +350,28 @@ class TestDomainCreation(MockEppLib):
     """Rule: An approved domain request must result in a domain"""
 
     @less_console_noise_decorator
-    def test_get_security_email_race_condition(self):
+    def test_get_or_create_public_contact_race_condition(self):
         """
         Scenario: Two processes try to create the same security contact simultaneously
             Given a domain in UNKNOWN state
             When a race condition occurs during contact creation
             Then no IntegrityError is raised
             And only one security contact exists in database
-            And the correct security email is returned
+            And the correct public contact is returned
+
+        CONTEXT: We ran into an intermittent but somewhat rare issue where IntegrityError
+        was raised when creating PublicContact.
+        Per our logs, this seemed to appear during periods of high app activity.
         """
         domain, _ = Domain.objects.get_or_create(name="defaultsecurity.gov")
-        
-        # Store original method
-        original_filter = PublicContact.objects.filter
-        self.first_call = True
-        def mock_filter(*args, **kwargs):
-            """ Simulates the race condition by creating a 
-            duplicate contact between filter check and save
-            """
-            result = original_filter(*args, **kwargs)
 
-            # Return empty queryset for first call. Otherwise just proceed as normal.
+        self.first_call = True
+
+        def mock_filter(*args, **kwargs):
+            """Simulates a race condition by creating a
+            duplicate contact between the first filter and save.
+            """
+            # Return an empty queryset for the first call. Otherwise just proceed as normal.
             if self.first_call:
                 self.first_call = False
                 duplicate = PublicContact(
@@ -378,21 +379,21 @@ class TestDomainCreation(MockEppLib):
                     contact_type=PublicContact.ContactTypeChoices.SECURITY,
                     registry_id="defaultSec",
                     email="dotgov@cisa.dhs.gov",
-                    name="Registry Customer Service"
+                    name="Registry Customer Service",
                 )
                 duplicate.save(skip_epp_save=True)
                 return PublicContact.objects.none()
 
-            return result
-            
-        with patch.object(PublicContact.objects, 'filter', side_effect=mock_filter):
+            return PublicContact.objects.filter(*args, **kwargs)
+
+        with patch.object(PublicContact.objects, "filter", side_effect=mock_filter):
             try:
                 public_contact = PublicContact(
                     domain=domain,
                     contact_type=PublicContact.ContactTypeChoices.SECURITY,
                     registry_id="defaultSec",
                     email="dotgov@cisa.dhs.gov",
-                    name="Registry Customer Service"
+                    name="Registry Customer Service",
                 )
                 returned_public_contact = domain._get_or_create_public_contact(public_contact)
             except IntegrityError:
@@ -403,13 +404,14 @@ class TestDomainCreation(MockEppLib):
                     "Expected behavior: gracefully handle duplicate creation and return existing contact."
                 )
 
-        # Verify only one contact exists
+        # Verify that only one contact exists and its correctness
         security_contacts = PublicContact.objects.filter(
-            domain=domain,
-            contact_type=PublicContact.ContactTypeChoices.SECURITY
+            domain=domain, contact_type=PublicContact.ContactTypeChoices.SECURITY
         )
         self.assertEqual(security_contacts.count(), 1)
         self.assertEqual(returned_public_contact, security_contacts.get())
+        self.assertEqual(returned_public_contact.registry_id, "defaultSec")
+        self.assertEqual(returned_public_contact.email, "dotgov@cisa.dhs.gov")
 
     @boto3_mocking.patching
     def test_approved_domain_request_creates_domain_locally(self):

From 8c204d4e8e2c392baddd35508dad83387a786358 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Fri, 17 Jan 2025 11:10:24 -0700
Subject: [PATCH 48/60] Remove unrelated changes

---
 src/registrar/admin.py         |  3 --
 src/registrar/models/domain.py | 56 +++++++++++++++++-----------------
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 8773f7ef8..849cb6100 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -3835,9 +3835,6 @@ class PublicContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
 
     change_form_template = "django/admin/email_clipboard_change_form.html"
     autocomplete_fields = ["domain"]
-    list_display = ("name", "contact_type", "email", "domain", "registry_id")
-    search_fields = ["email", "name", "registry_id"]
-    search_help_text = "Search by email, name or registry id."
 
     def changeform_view(self, request, object_id=None, form_url="", extra_context=None):
         if extra_context is None:
diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 2ace5f22d..d024efe5f 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1665,6 +1665,30 @@ class Domain(TimeStampedModel, DomainHelper):
                 )
             return err.code
 
+    def _fetch_contacts(self, contact_data):
+        """Fetch contact info."""
+        choices = PublicContact.ContactTypeChoices
+        # We expect that all these fields get populated,
+        # so we can create these early, rather than waiting.
+        contacts_dict = {
+            choices.ADMINISTRATIVE: None,
+            choices.SECURITY: None,
+            choices.TECHNICAL: None,
+        }
+        for domainContact in contact_data:
+            req = commands.InfoContact(id=domainContact.contact)
+            data = registry.send(req, cleaned=True).res_data[0]
+            logger.info(f"_fetch_contacts => this is the data: {data}")
+
+            # Map the object we recieved from EPP to a PublicContact
+            mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type)
+            logger.info(f"_fetch_contacts => mapped_object: {mapped_object}")
+
+            # Find/create it in the DB
+            in_db = self._get_or_create_public_contact(mapped_object)
+            contacts_dict[in_db.contact_type] = in_db.registry_id
+        return contacts_dict
+
     def _get_or_create_contact(self, contact: PublicContact):
         """Try to fetch info about a contact. Create it if it does not exist."""
         logger.info("_get_or_create_contact() -> Fetching contact info")
@@ -1826,6 +1850,7 @@ class Domain(TimeStampedModel, DomainHelper):
         """
         try:
             self._add_missing_contacts_if_unknown(cleaned)
+
         except Exception as e:
             logger.error(
                 "%s couldn't _add_missing_contacts_if_unknown, error was %s."
@@ -1843,6 +1868,7 @@ class Domain(TimeStampedModel, DomainHelper):
         is in an UNKNOWN state, that is an error state)
         Note: The transition state change happens at the end of the function
         """
+
         missingAdmin = True
         missingSecurity = True
         missingTech = True
@@ -1938,8 +1964,6 @@ class Domain(TimeStampedModel, DomainHelper):
         Additionally, capture and cache old hosts and contacts from cache if they
         don't exist in cleaned
         """
-
-        # object reference issue between self._cache vs cleaned?
         old_cache_hosts = self._cache.get("hosts")
         old_cache_contacts = self._cache.get("contacts")
 
@@ -2052,30 +2076,6 @@ class Domain(TimeStampedModel, DomainHelper):
             cleaned_contacts = self._fetch_contacts(contacts)
         return cleaned_contacts
 
-    def _fetch_contacts(self, contact_data):
-        """Fetch contact info."""
-        choices = PublicContact.ContactTypeChoices
-        # We expect that all these fields get populated,
-        # so we can create these early, rather than waiting.
-        contacts_dict = {
-            choices.ADMINISTRATIVE: None,
-            choices.SECURITY: None,
-            choices.TECHNICAL: None,
-        }
-        for domainContact in contact_data:
-            req = commands.InfoContact(id=domainContact.contact)
-            data = registry.send(req, cleaned=True).res_data[0]
-            logger.info(f"_fetch_contacts => this is the data: {data}")
-
-            # Map the object we recieved from EPP to a PublicContact
-            mapped_object = self.map_epp_contact_to_public_contact(data, domainContact.contact, domainContact.type)
-            logger.info(f"_fetch_contacts => mapped_object: {mapped_object}")
-
-            # Find/create it in the DB
-            in_db = self._get_or_create_public_contact(mapped_object)
-            contacts_dict[in_db.contact_type] = in_db.registry_id
-        return contacts_dict
-
     def _get_hosts(self, hosts):
         cleaned_hosts = []
         if hosts and isinstance(hosts, list):
@@ -2118,7 +2118,7 @@ class Domain(TimeStampedModel, DomainHelper):
                     logger.info(f"Created a new PublicContact: {public_contact}")
             except IntegrityError as err:
                 logger.error(
-                    "_get_or_create_public_contact() => tried to create a duplicate public contact: " f"{err}",
+                    f"_get_or_create_public_contact() => tried to create a duplicate public contact: {err}",
                     exc_info=True,
                 )
                 return PublicContact.objects.get(
@@ -2136,7 +2136,7 @@ class Domain(TimeStampedModel, DomainHelper):
         if existing_contact.email != public_contact.email or existing_contact.registry_id != public_contact.registry_id:
             existing_contact.delete()
             public_contact.save()
-            logger.warning("Requested PublicContact is out of sync with our DB.")
+            logger.warning("Requested PublicContact is out of sync with DB.")
             return public_contact
 
         # If it already exists, we can assume that the DB instance was updated during set, so we should just use that.

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 49/60] 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 ba2788014b5118364c1c0054b3587e68e56d7601 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 17 Jan 2025 17:38:00 -0500
Subject: [PATCH 50/60] removed default empty options from Checkboxes as they
 are only needed for Selects

---
 src/registrar/assets/src/js/getgov/requesting-entity.js | 3 +--
 src/registrar/forms/domain.py                           | 3 +--
 src/registrar/forms/domain_request_wizard.py            | 9 +++------
 src/registrar/forms/portfolio.py                        | 2 +-
 4 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov/requesting-entity.js b/src/registrar/assets/src/js/getgov/requesting-entity.js
index 5f3be8c79..833eab2f8 100644
--- a/src/registrar/assets/src/js/getgov/requesting-entity.js
+++ b/src/registrar/assets/src/js/getgov/requesting-entity.js
@@ -15,7 +15,6 @@ export function handleRequestingEntityFieldset() {
     const suborgContainer = document.getElementById("suborganization-container");
     const suborgDetailsContainer = document.getElementById("suborganization-container__details");
     const suborgAddtlInstruction = document.getElementById("suborganization-addtl-instruction");
-    const subOrgCreateNewOption = document.getElementById("option-to-add-suborg")?.value;
     // Make sure all crucial page elements exist before proceeding.
     // This more or less ensures that we are on the Requesting Entity page, and not elsewhere.
     if (!radios || !input || !select || !inputGrandParent || !suborgContainer || !suborgDetailsContainer) return;
@@ -28,7 +27,7 @@ export function handleRequestingEntityFieldset() {
     function toggleSuborganization(radio=null) {
         if (radio != null) requestingSuborganization = radio?.checked && radio.value === "True";
         requestingSuborganization ? showElement(suborgContainer) : hideElement(suborgContainer);
-        if (select.options.length == 2) { // --Select-- and other are the only options
+        if (select.options.length == 1) { // other is the only option
             hideElement(inputGrandParent); // Hide the combo box and indicate requesting new suborg
             hideElement(suborgAddtlInstruction); // Hide additional instruction related to the list
             requestingNewSuborganization.value = "True";
diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index b3dae0d3a..5eeae232d 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -450,7 +450,6 @@ class DomainOrgNameAddressForm(forms.ModelForm):
         label="Federal agency",
         required=False,
         queryset=FederalAgency.objects.all(),
-        empty_label="--Select--",
         widget=ComboboxWidget,
     )
     zipcode = forms.CharField(
@@ -469,7 +468,7 @@ class DomainOrgNameAddressForm(forms.ModelForm):
     state_territory = forms.ChoiceField(
         label="State, territory, or military post",
         required=True,
-        choices=[("", "--Select--")] + DomainInformation.StateTerritoryChoices.choices,
+        choices=DomainInformation.StateTerritoryChoices.choices,
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 12d3b74bf..7f80c9d34 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -44,7 +44,6 @@ class RequestingEntityForm(RegistrarForm):
         label="Suborganization name",
         required=False,
         queryset=Suborganization.objects.none(),
-        empty_label="--Select--",
         widget=ComboboxWidget,
     )
     requested_suborganization = forms.CharField(
@@ -58,7 +57,7 @@ class RequestingEntityForm(RegistrarForm):
     suborganization_state_territory = forms.ChoiceField(
         label="State, territory, or military post",
         required=False,
-        choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices,
+        choices=DomainRequest.StateTerritoryChoices.choices,
         widget=ComboboxWidget,
     )
 
@@ -74,8 +73,7 @@ class RequestingEntityForm(RegistrarForm):
 
             # Modify the choices to include "other" so that form can display options properly
             self.fields["sub_organization"].choices = (
-                [("", "--Select--")]
-                + [(obj.id, str(obj)) for obj in queryset]
+                [(obj.id, str(obj)) for obj in queryset]
                 + [("other", "Other (enter your suborganization manually)")]
             )
 
@@ -328,7 +326,6 @@ class OrganizationContactForm(RegistrarForm):
         # uncomment to see if modelChoiceField can be an arg later
         required=False,
         queryset=FederalAgency.objects.exclude(agency__in=excluded_agencies),
-        empty_label="--Select--",
         widget=ComboboxWidget,
     )
     organization_name = forms.CharField(
@@ -349,7 +346,7 @@ class OrganizationContactForm(RegistrarForm):
     )
     state_territory = forms.ChoiceField(
         label="State, territory, or military post",
-        choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices,
+        choices=DomainRequest.StateTerritoryChoices.choices,
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 13d956fe4..9d6c9ecdf 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -37,7 +37,7 @@ class PortfolioOrgAddressForm(forms.ModelForm):
     state_territory = forms.ChoiceField(
         label="State, territory, or military post",
         required=True,
-        choices=[("", "--Select--")] + DomainInformation.StateTerritoryChoices.choices,
+        choices=DomainInformation.StateTerritoryChoices.choices,
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },

From 463632baf769c901c1cec198fdcda293744d632d Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 17 Jan 2025 17:42:00 -0500
Subject: [PATCH 51/60] lint

---
 src/registrar/forms/domain_request_wizard.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 7f80c9d34..e67595c21 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -72,10 +72,9 @@ class RequestingEntityForm(RegistrarForm):
             self.fields["sub_organization"].queryset = queryset
 
             # Modify the choices to include "other" so that form can display options properly
-            self.fields["sub_organization"].choices = (
-                [(obj.id, str(obj)) for obj in queryset]
-                + [("other", "Other (enter your suborganization manually)")]
-            )
+            self.fields["sub_organization"].choices = [(obj.id, str(obj)) for obj in queryset] + [
+                ("other", "Other (enter your suborganization manually)")
+            ]
 
     @classmethod
     def from_database(cls, obj: DomainRequest | Contact | None):

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 52/60] 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 53/60] 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 96618ee2bc6ff7d1a2a9cee33cac68d38ec10679 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 17 Jan 2025 20:03:04 -0500
Subject: [PATCH 54/60] fixing tests

---
 src/registrar/forms/domain_request_wizard.py |  3 --
 src/registrar/tests/test_views_portfolio.py  | 30 +++++++++++++++-----
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index e67595c21..7c9dcb180 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/src/registrar/forms/domain_request_wizard.py
@@ -95,7 +95,6 @@ class RequestingEntityForm(RegistrarForm):
     def clean_sub_organization(self):
         """On suborganization clean, set the suborganization value to None if the user is requesting
         a custom suborganization (as it doesn't exist yet)"""
-
         # If it's a new suborganization, return None (equivalent to selecting nothing)
         if self.cleaned_data.get("is_requesting_new_suborganization"):
             return None
@@ -129,7 +128,6 @@ class RequestingEntityForm(RegistrarForm):
             # Retrieve sub_organization and store in _original_suborganization
             suborganization = data.get("portfolio_requesting_entity-sub_organization")
             self._original_suborganization = suborganization
-
             # If the original value was "other", clear it for validation
             if self._original_suborganization == "other":
                 data["portfolio_requesting_entity-sub_organization"] = ""
@@ -171,7 +169,6 @@ class RequestingEntityForm(RegistrarForm):
         requesting_entity_is_suborganization = self.data.get(
             "portfolio_requesting_entity-requesting_entity_is_suborganization"
         )
-
         if requesting_entity_is_suborganization == "True":
             if is_requesting_new_suborganization:
                 if not cleaned_data.get("requested_suborganization") and "requested_suborganization" not in self.errors:
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index 78a4dae82..e2de3328f 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -2879,7 +2879,7 @@ class TestRequestingEntity(WebTest):
 
         form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
         form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True
-        form["portfolio_requesting_entity-sub_organization"] = ""
+        form["portfolio_requesting_entity-sub_organization"] = "other"
 
         form["portfolio_requesting_entity-requested_suborganization"] = "moon"
         form["portfolio_requesting_entity-suborganization_city"] = "kepler"
@@ -2933,7 +2933,7 @@ class TestRequestingEntity(WebTest):
 
     @override_flag("organization_feature", active=True)
     @override_flag("organization_requests", active=True)
-    @less_console_noise_decorator
+    # @less_console_noise_decorator
     def test_requesting_entity_page_errors(self):
         """Tests that we get the expected form errors on requesting entity"""
         domain_request = completed_domain_request(user=self.user, portfolio=self.portfolio)
@@ -2942,18 +2942,34 @@ class TestRequestingEntity(WebTest):
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
 
+        # For 2 the tests below, it is required to submit a form without submitting a value
+        # for the select/combobox. WebTest will not do this; by default, WebTest will submit
+        # the first choice in a select. So, need to manipulate the form to remove the
+        # particular select/combobox that will not be submitted, and then post the form.
+        form_action = f"/request/{domain_request.pk}/portfolio_requesting_entity/"
+
         # Test missing suborganization selection
         form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
-        form["portfolio_requesting_entity-sub_organization"] = ""
-
-        response = form.submit()
+        form["portfolio_requesting_entity-is_requesting_new_suborganization"] = False
+        # remove sub_organization from the form submission
+        form_data = form.submit_fields()
+        form_data = [(key, value) for key, value in form_data if key != "portfolio_requesting_entity-sub_organization"]
+        response = self.app.post(form_action, dict(form_data))
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
         self.assertContains(response, "Suborganization is required.", status_code=200)
 
         # Test missing custom suborganization details
+        form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
         form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True
-        response = form.submit()
-        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        form["portfolio_requesting_entity-sub_organization"] = "other"
+        # remove suborganization_state_territory from the form submission
+        form_data = form.submit_fields()
+        form_data = [
+            (key, value)
+            for key, value in form_data
+            if key != "portfolio_requesting_entity-suborganization_state_territory"
+        ]
+        response = self.app.post(form_action, dict(form_data))
         self.assertContains(response, "Enter the name of your suborganization.", status_code=200)
         self.assertContains(response, "Enter the city where your suborganization is located.", status_code=200)
         self.assertContains(

From 6bf52f00150daeaee062c795382cb8f07282c0f7 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 20 Jan 2025 07:10:12 -0500
Subject: [PATCH 55/60] cleanup

---
 src/registrar/forms/domain.py               | 1 -
 src/registrar/tests/test_views_portfolio.py | 2 +-
 src/registrar/views/domain_request.py       | 1 +
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 5eeae232d..711315632 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -164,7 +164,6 @@ class DomainSuborganizationForm(forms.ModelForm):
     sub_organization = forms.ModelChoiceField(
         label="Suborganization name",
         queryset=Suborganization.objects.none(),
-        empty_label="⎯ (No suborganization)",
         required=False,
         widget=ComboboxWidget,
     )
diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py
index e2de3328f..69502d683 100644
--- a/src/registrar/tests/test_views_portfolio.py
+++ b/src/registrar/tests/test_views_portfolio.py
@@ -2933,7 +2933,7 @@ class TestRequestingEntity(WebTest):
 
     @override_flag("organization_feature", active=True)
     @override_flag("organization_requests", active=True)
-    # @less_console_noise_decorator
+    @less_console_noise_decorator
     def test_requesting_entity_page_errors(self):
         """Tests that we get the expected form errors on requesting entity"""
         domain_request = completed_domain_request(user=self.user, portfolio=self.portfolio)
diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py
index bff3e5c00..9754b0d0c 100644
--- a/src/registrar/views/domain_request.py
+++ b/src/registrar/views/domain_request.py
@@ -368,6 +368,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView):
         and from the database if `use_db` is True (provided that record exists).
         An empty form will be provided if neither of those are true.
         """
+
         kwargs = {
             "files": files,
             "prefix": self.steps.current,

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 56/60] 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 57/60] 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']"
                 ),
             )
 

From e9268bea8b3883a6ae62d578617b168c5f36d107 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 22 Jan 2025 13:17:22 -0500
Subject: [PATCH 58/60] updated required attribute on state territory

---
 src/registrar/forms/domain.py    | 2 +-
 src/registrar/forms/portfolio.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 711315632..20091a325 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -471,7 +471,7 @@ class DomainOrgNameAddressForm(forms.ModelForm):
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
-        widget=ComboboxWidget(),
+        widget=ComboboxWidget(attrs={"required":True}),
     )
 
     class Meta:
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 9d6c9ecdf..379e8053c 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -41,7 +41,7 @@ class PortfolioOrgAddressForm(forms.ModelForm):
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
-        widget=ComboboxWidget,
+        widget=ComboboxWidget(attrs={"required":True}),
     )
 
     class Meta:

From 5a30d28b34307e62432e8a7ed841baaa921b03a6 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 22 Jan 2025 13:20:11 -0700
Subject: [PATCH 59/60] cleanup logs

---
 src/registrar/models/domain.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index d024efe5f..cb481db7a 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -1329,14 +1329,14 @@ class Domain(TimeStampedModel, DomainHelper):
 
     def get_default_administrative_contact(self):
         """Gets the default administrative contact."""
-        logger.info("get_default_administrative_contact() -> Adding administrative security contact")
+        logger.info("get_default_administrative_contact() -> Adding default administrative contact")
         contact = PublicContact.get_default_administrative()
         contact.domain = self
         return contact
 
     def get_default_technical_contact(self):
         """Gets the default technical contact."""
-        logger.info("get_default_security_contact() -> Adding technical security contact")
+        logger.info("get_default_security_contact() -> Adding default technical contact")
         contact = PublicContact.get_default_technical()
         contact.domain = self
         return contact

From 6828a1f1ff022a4da0c58b8e17f34d1d6320f439 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 22 Jan 2025 15:33:08 -0500
Subject: [PATCH 60/60] lint

---
 src/registrar/forms/domain.py    | 2 +-
 src/registrar/forms/portfolio.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py
index 20091a325..05eb90db3 100644
--- a/src/registrar/forms/domain.py
+++ b/src/registrar/forms/domain.py
@@ -471,7 +471,7 @@ class DomainOrgNameAddressForm(forms.ModelForm):
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
-        widget=ComboboxWidget(attrs={"required":True}),
+        widget=ComboboxWidget(attrs={"required": True}),
     )
 
     class Meta:
diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py
index 379e8053c..e57b56c4f 100644
--- a/src/registrar/forms/portfolio.py
+++ b/src/registrar/forms/portfolio.py
@@ -41,7 +41,7 @@ class PortfolioOrgAddressForm(forms.ModelForm):
         error_messages={
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
-        widget=ComboboxWidget(attrs={"required":True}),
+        widget=ComboboxWidget(attrs={"required": True}),
     )
 
     class Meta: