diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ae06a8d96..31cbdc38d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -22,7 +22,7 @@ from epplibwrapper.errors import ErrorCode, RegistryError from registrar.models.user_domain_role import UserDomainRole from waffle.admin import FlagAdmin from waffle.models import Sample, Switch -from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website +from registrar.models import Contact, Domain, DomainRequest, DraftDomain, User, Website, SeniorOfficial from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.views.utility.mixins import OrderableFieldsMixin from django.contrib.admin.views.main import ORDER_VAR @@ -493,6 +493,8 @@ class CustomLogEntryAdmin(LogEntryAdmin): # return super().change_view(request, object_id, form_url, extra_context=extra_context) +# TODO - this should be refactored. This is shared among every class that inherits this, +# and it breaks the senior_official field because it exists both as model "Contact" and "SeniorOfficial". class AdminSortFields: _name_sort = ["first_name", "last_name", "email"] @@ -504,6 +506,8 @@ class AdminSortFields: # == Contact == # "other_contacts": (Contact, _name_sort), "submitter": (Contact, _name_sort), + # == Senior Official == # + "senior_official": (SeniorOfficial, _name_sort), # == User == # "creator": (User, _name_sort), "user": (User, _name_sort), @@ -553,15 +557,16 @@ class AuditedAdmin(admin.ModelAdmin): ) ) - def formfield_for_manytomany(self, db_field, request, **kwargs): + def formfield_for_manytomany(self, db_field, request, use_admin_sort_fields=True, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" # Define a queryset. Note that in the super of this, # a new queryset will only be generated if one does not exist. # Thus, the order in which we define queryset matters. + queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and use_admin_sort_fields: kwargs["queryset"] = queryset formfield = super().formfield_for_manytomany(db_field, request, **kwargs) @@ -572,7 +577,7 @@ class AuditedAdmin(admin.ModelAdmin): ) return formfield - def formfield_for_foreignkey(self, db_field, request, **kwargs): + def formfield_for_foreignkey(self, db_field, request, use_admin_sort_fields=True, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" @@ -580,7 +585,7 @@ class AuditedAdmin(admin.ModelAdmin): # a new queryset will only be generated if one does not exist. # Thus, the order in which we define queryset matters. queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and use_admin_sort_fields: kwargs["queryset"] = queryset return super().formfield_for_foreignkey(db_field, request, **kwargs) @@ -1542,6 +1547,16 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): # Get the filtered values return super().changelist_view(request, extra_context=extra_context) + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. + use_sort = db_field.name != "senior_official" + return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + class DomainRequestResource(FsmModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the @@ -2209,6 +2224,16 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): return None + def formfield_for_foreignkey(self, db_field, request, **kwargs): + """Customize the behavior of formfields with foreign key relationships. This will customize + the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. + use_sort = db_field.name != "senior_official" + return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs) + class TransitionDomainAdmin(ListHeaderAdmin): """Custom transition domain admin class.""" @@ -2258,6 +2283,7 @@ class DomainInformationInline(admin.StackedInline): def formfield_for_manytomany(self, db_field, request, **kwargs): """customize the behavior of formfields with manytomany relationships. the customized behavior includes sorting of objects in lists as well as customizing helper text""" + queryset = AdminSortFields.get_queryset(db_field) if queryset: kwargs["queryset"] = queryset @@ -2272,8 +2298,12 @@ class DomainInformationInline(admin.StackedInline): def formfield_for_foreignkey(self, db_field, request, **kwargs): """Customize the behavior of formfields with foreign key relationships. This will customize the behavior of selects. Customized behavior includes sorting of objects in list.""" + # Remove this check on senior_official if this underlying model changes from + # "Contact" to "SeniorOfficial" or if we refactor AdminSortFields. + # Removing this will cause the list on django admin to return SeniorOffical + # objects rather than Contact objects. queryset = AdminSortFields.get_queryset(db_field) - if queryset: + if queryset and db_field.name != "senior_official": kwargs["queryset"] = queryset return super().formfield_for_foreignkey(db_field, request, **kwargs) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 4a1e95431..a46a4d3e8 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -326,7 +326,7 @@ class SeniorOfficialContactForm(ContactForm): def __init__(self, disable_fields=False, *args, **kwargs): super().__init__(*args, **kwargs) - if self.instance: + if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() # Overriding bc phone not required in this form diff --git a/src/registrar/forms/portfolio.py b/src/registrar/forms/portfolio.py index 44d195dd6..67ccf2464 100644 --- a/src/registrar/forms/portfolio.py +++ b/src/registrar/forms/portfolio.py @@ -87,5 +87,5 @@ class PortfolioSeniorOfficialForm(forms.ModelForm): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - if self.instance: + if self.instance and self.instance.id: self.fields["full_name"].initial = self.instance.get_formatted_name() diff --git a/src/registrar/templates/includes/senior_official.html b/src/registrar/templates/includes/senior_official.html index c63493ae8..b080127f4 100644 --- a/src/registrar/templates/includes/senior_official.html +++ b/src/registrar/templates/includes/senior_official.html @@ -44,4 +44,4 @@ {% if form.email.value is not None %} {% include "includes/input_read_only.html" with field=form.email %} {% endif %} -{% endif %} \ No newline at end of file +{% endif %} diff --git a/src/registrar/tests/test_views_portfolio.py b/src/registrar/tests/test_views_portfolio.py index d725c95ad..60764cf1c 100644 --- a/src/registrar/tests/test_views_portfolio.py +++ b/src/registrar/tests/test_views_portfolio.py @@ -41,7 +41,7 @@ class TestPortfolio(WebTest): @less_console_noise_decorator @override_flag("organization_feature", active=True) def test_portfolio_senior_official(self): - """Tests the senior official page on portfolio""" + """Tests that the senior official page on portfolio contains the content we expect""" self.app.set_user(self.user.username) so = SeniorOfficial.objects.create( @@ -63,6 +63,7 @@ class TestPortfolio(WebTest): self.assertContains(so_portfolio_page, "Saturn Enceladus") self.assertContains(so_portfolio_page, "Planet/Moon") self.assertContains(so_portfolio_page, "spacedivision@igorville.com") + self.assertNotContains(so_portfolio_page, "Save") self.portfolio.delete() so.delete()