From 9f9147e66181795ded63c97f0523199800dff2bb Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 1 Nov 2024 17:37:44 -0600 Subject: [PATCH 01/29] Initial draft --- src/registrar/admin.py | 15 +++++++++++++-- src/registrar/models/domain.py | 2 +- src/registrar/models/domain_information.py | 11 +++++++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8a0a458f8..09c36d137 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1461,10 +1461,15 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): form = DomainInformationAdminForm + # Customize column header text + @admin.display(description=_("Generic Org Type")) + def converted_generic_org_type(self, obj): + return obj.converted_generic_org_type + # Columns list_display = [ "domain", - "generic_org_type", + "converted_generic_org_type", "created_at", ] @@ -1493,7 +1498,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): }, ), (".gov domain", {"fields": ["domain"]}), - ("Contacts", {"fields": ["senior_official", "other_contacts", "no_other_contacts_rationale"]}), + ("Contacts", {"fields": ["generic_org_type", "other_contacts", "no_other_contacts_rationale"]}), ("Background info", {"fields": ["anything_else"]}), ( "Type of organization", @@ -1611,6 +1616,12 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): # 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) + + def get_queryset(self, request): + qs = super().get_queryset(request) + return qs.annotate( + converted_generic_org_type_display="hey" + ) class DomainRequestResource(FsmModelResource): diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7fdc56971..339e4dd20 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2078,4 +2078,4 @@ def _get_property(self, property): if property in self._cache: return self._cache[property] else: - raise KeyError("Requested key %s was not found in registry cache." % str(property)) + raise KeyError("Requested key %s was not found in registry cache." % str(property)) \ No newline at end of file diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 7dadf26ac..525d7998e 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -426,13 +426,14 @@ def get_state_display_of_domain(self): else: return None + # ----- Portfolio Properties ----- + @property def converted_organization_name(self): if self.portfolio: return self.portfolio.organization_name return self.organization_name - # ----- Portfolio Properties ----- @property def converted_generic_org_type(self): if self.portfolio: @@ -474,7 +475,7 @@ def converted_city(self): if self.portfolio: return self.portfolio.city return self.city - + @property def converted_state_territory(self): if self.portfolio: @@ -492,3 +493,9 @@ def converted_urbanization(self): if self.portfolio: return self.portfolio.urbanization return self.urbanization + + + + + + From 7292926f60009b217c74c8d9e5ed2fe346e43ac7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 6 Nov 2024 14:33:07 -0700 Subject: [PATCH 02/29] WIP on nl/2975-domain-and-domain-info-portfolio-fields Filter --- src/registrar/admin.py | 41 ++++++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 09c36d137..7cb4cb49e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1457,12 +1457,41 @@ class Meta: class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Customize domain information admin class.""" + class GenericOrgFilter(admin.SimpleListFilter): + """Custom Generic Organization filter that accomodates portfolio feature. + If we have a portfolio, use the portfolio's organization. If not, use the + organization in the Domain Information object.""" + + title = "generic organization" + parameter_name = 'converted_generic_orgs' + + def lookups(self, request, model_admin): + converted_generic_orgs = set() + + for domainInfo in DomainInformation.objects.all(): + converted_generic_org = domainInfo.converted_generic_org_type + if converted_generic_org: + converted_generic_orgs.add(converted_generic_org) + + return sorted((org, org) for org in converted_generic_orgs) + + # Filter queryset + def queryset(self, request, queryset): + if self.value(): # Check if a generic org is selected in the filter + return queryset.filter( + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domainInfo.id for domainInfo in queryset if domainInfo.converted_generic_org_type and domainInfo.converted_generic_org_type == self.value() + ] + ) + return queryset + resource_classes = [DomainInformationResource] form = DomainInformationAdminForm # Customize column header text - @admin.display(description=_("Generic Org Type")) + @admin.display(description=_("Converted Generic Org Type")) def converted_generic_org_type(self, obj): return obj.converted_generic_org_type @@ -1476,7 +1505,7 @@ def converted_generic_org_type(self, obj): orderable_fk_fields = [("domain", "name")] # Filters - list_filter = ["generic_org_type"] + list_filter = [GenericOrgFilter] # Search search_fields = [ @@ -1550,7 +1579,7 @@ def converted_generic_org_type(self, obj): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board") + readonly_fields = ("other_contacts", "is_election_board", "converted_generic_org_type") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ @@ -1616,12 +1645,6 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): # 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) - - def get_queryset(self, request): - qs = super().get_queryset(request) - return qs.annotate( - converted_generic_org_type_display="hey" - ) class DomainRequestResource(FsmModelResource): From 0ac4e5766cc67fcfa97c87a4f15fee996ca0ecb7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 6 Nov 2024 14:33:46 -0700 Subject: [PATCH 03/29] cleanup stray readonly field --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7cb4cb49e..0819aefd2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1579,7 +1579,7 @@ def converted_generic_org_type(self, obj): ] # Readonly fields for analysts and superusers - readonly_fields = ("other_contacts", "is_election_board", "converted_generic_org_type") + readonly_fields = ("other_contacts", "is_election_board") # Read only that we'll leverage for CISA Analysts analyst_readonly_fields = [ From b0e1b5a14987ec7a7544374eea428cbcaa628a04 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 6 Nov 2024 16:40:07 -0700 Subject: [PATCH 04/29] updates to DomainAdmin --- src/registrar/admin.py | 170 +++++++++++++++++++++++++++++++++-------- 1 file changed, 137 insertions(+), 33 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0819aefd2..e32a74414 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2495,6 +2495,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): resource_classes = [DomainResource] + # ------- FILTERS class ElectionOfficeFilter(admin.SimpleListFilter): """Define a custom filter for is_election_board""" @@ -2512,19 +2513,96 @@ def queryset(self, request, queryset): return queryset.filter(domain_info__is_election_board=True) if self.value() == "0": return queryset.filter(Q(domain_info__is_election_board=False) | Q(domain_info__is_election_board=None)) + + class GenericOrgFilter(admin.SimpleListFilter): + """Custom Generic Organization filter that accomodates portfolio feature. + If we have a portfolio, use the portfolio's organization. If not, use the + organization in the Domain Information object.""" + + title = "generic organization" + parameter_name = 'converted_generic_orgs' + + def lookups(self, request, model_admin): + converted_generic_orgs = set() + + for domainInfo in DomainInformation.objects.all(): + converted_generic_org = domainInfo.converted_generic_org_type + if converted_generic_org: + converted_generic_orgs.add(converted_generic_org) + + return sorted((org, org) for org in converted_generic_orgs) + + # Filter queryset + def queryset(self, request, queryset): + if self.value(): # Check if a generic org is selected in the filter + return queryset.filter( + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domain.id for domain in queryset if domain.domain_info.converted_generic_org_type and domain.domain_info.converted_generic_org_type == self.value() + ] + ) + return queryset + + class FederalTypeFilter(admin.SimpleListFilter): + """Custom Federal Type filter that accomodates portfolio feature. + If we have a portfolio, use the portfolio's federal type. If not, use the + federal type in the Domain Information object.""" + + title = "federal type" + parameter_name = 'converted_federal_types' + + def lookups(self, request, model_admin): + converted_federal_types = set() + # converted_federal_types.add("blah") + + for domainInfo in DomainInformation.objects.all(): + converted_federal_type = domainInfo.converted_federal_type + if converted_federal_type: + converted_federal_types.add(converted_federal_type) + + return sorted((fed, fed) for fed in converted_federal_types) + + # Filter queryset + def queryset(self, request, queryset): + if self.value(): # Check if a generic org is selected in the filter + return queryset.filter( + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domain.id for domain in queryset if domain.domain_info.converted_federal_type and domain.domain_info.converted_federal_type == self.value() + ] + ) + return queryset + + def get_queryset(self, request): + """Custom get_queryset to filter by portfolio if portfolio is in the + request params.""" + qs = super().get_queryset(request) + # Check if a 'portfolio' parameter is passed in the request + portfolio_id = request.GET.get("portfolio") + if portfolio_id: + # Further filter the queryset by the portfolio + qs = qs.filter(domain_info__portfolio=portfolio_id) + return qs + + # Filters + list_filter = [GenericOrgFilter, FederalTypeFilter, ElectionOfficeFilter, "state"] + + # ------- END FILTERS + + # Inlines inlines = [DomainInformationInline] # Columns list_display = [ "name", - "generic_org_type", + "converted_generic_org_type", "federal_type", - "federal_agency", - "organization_name", + "converted_federal_agency", + "converted_organization_name", "custom_election_board", - "city", - "state_territory", + "converted_city", + "converted_state_territory", "state", "expiration_date", "created_at", @@ -2539,28 +2617,71 @@ def queryset(self, request, queryset): ), ) + # ------- Domain Information Fields + + # --- Generic Org Type + @admin.display(description=_("Converted Generic Org Type")) + def converted_generic_org_type(self, obj): + return obj.domain_info.converted_generic_org_type + converted_generic_org_type.admin_order_field = "domain_info__converted_generic_org_type" # type: ignore + def generic_org_type(self, obj): return obj.domain_info.get_generic_org_type_display() + # generic_org_type.admin_order_field = "domain_info__generic_org_type" # type: ignore - generic_org_type.admin_order_field = "domain_info__generic_org_type" # type: ignore + # --- Federal Agency + @admin.display(description=_("Converted Federal Agency")) + def converted_federal_agency(self, obj): + return obj.domain_info.converted_federal_agency + converted_federal_agency.admin_order_field = "domain_info__converted_federal_agency" # type: ignore def federal_agency(self, obj): if obj.domain_info: return obj.domain_info.federal_agency else: return None + # federal_agency.admin_order_field = "domain_info__federal_agency" # type: ignore - federal_agency.admin_order_field = "domain_info__federal_agency" # type: ignore + # --- Federal Type + @admin.display(description=_("Converted Federal Type")) + def converted_federal_type(self, obj): + return obj.domain_info.converted_federal_type + converted_federal_type.admin_order_field = "domain_info__converted_federal_type" # type: ignore def federal_type(self, obj): return obj.domain_info.federal_type if obj.domain_info else None + # federal_type.admin_order_field = "domain_info__federal_type" # type: ignore - federal_type.admin_order_field = "domain_info__federal_type" # type: ignore + # --- Organization Name + @admin.display(description=_("Converted Organization Name")) + def converted_organization_name(self, obj): + return obj.domain_info.converted_organization_name + converted_organization_name.admin_order_field = "domain_info__converted_organization_name" # type: ignore def organization_name(self, obj): return obj.domain_info.organization_name if obj.domain_info else None + # organization_name.admin_order_field = "domain_info__organization_name" # type: ignore + + # --- City + @admin.display(description=_("Converted City")) + def converted_city(self, obj): + return obj.domain_info.converted_city + converted_city.admin_order_field = "domain_info__converted_city" # type: ignore + + def city(self, obj): + return obj.domain_info.city if obj.domain_info else None + # city.admin_order_field = "domain_info__city" # type: ignore + + # --- State + @admin.display(description=_("Converted State / territory")) + def converted_state_territory(self, obj): + return obj.domain_info.converted_state_territory + converted_state_territory.admin_order_field = "domain_info__converted_state_territory" # type: ignore + + def state_territory(self, obj): + return obj.domain_info.state_territory if obj.domain_info else None + # state_territory.admin_order_field = "domain_info__state_territory" # type: ignore - organization_name.admin_order_field = "domain_info__organization_name" # type: ignore def dnssecdata(self, obj): return "Yes" if obj.dnssecdata else "No" @@ -2593,29 +2714,21 @@ def custom_election_board(self, obj): custom_election_board.admin_order_field = "domain_info__is_election_board" # type: ignore custom_election_board.short_description = "Election office" # type: ignore - def city(self, obj): - return obj.domain_info.city if obj.domain_info else None - - city.admin_order_field = "domain_info__city" # type: ignore - - @admin.display(description=_("State / territory")) - def state_territory(self, obj): - return obj.domain_info.state_territory if obj.domain_info else None - - state_territory.admin_order_field = "domain_info__state_territory" # type: ignore - - # Filters - list_filter = ["domain_info__generic_org_type", "domain_info__federal_type", ElectionOfficeFilter, "state"] + # Search search_fields = ["name"] search_help_text = "Search by domain name." + + # Change Form change_form_template = "django/admin/domain_change_form.html" + + # Readonly Fields readonly_fields = ( "state", "expiration_date", "first_ready", "deleted", - "federal_agency", + "converted_federal_agency", "dnssecdata", "nameservers", ) @@ -2871,16 +2984,7 @@ def has_change_permission(self, request, obj=None): return True return super().has_change_permission(request, obj) - def get_queryset(self, request): - """Custom get_queryset to filter by portfolio if portfolio is in the - request params.""" - qs = super().get_queryset(request) - # Check if a 'portfolio' parameter is passed in the request - portfolio_id = request.GET.get("portfolio") - if portfolio_id: - # Further filter the queryset by the portfolio - qs = qs.filter(domain_info__portfolio=portfolio_id) - return qs + class DraftDomainResource(resources.ModelResource): From ae0eb452cdc843cf857108a1c9a14ef2ada1b014 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 7 Nov 2024 11:01:08 -0700 Subject: [PATCH 05/29] revert read-only field --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e32a74414..8a591d8da 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2728,7 +2728,7 @@ def custom_election_board(self, obj): "expiration_date", "first_ready", "deleted", - "converted_federal_agency", + "federal_agency", "dnssecdata", "nameservers", ) From 32232c5d56dda556eff4cf759b37a77db49fd3b0 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 13 Nov 2024 01:16:55 -0700 Subject: [PATCH 06/29] export updates --- src/registrar/admin.py | 149 ++++++++++++++++++++- src/registrar/config/urls.py | 5 - src/registrar/models/domain_information.py | 8 +- src/registrar/models/domain_request.py | 12 +- src/registrar/utility/csv_export.py | 110 ++++++++++----- src/registrar/views/report_views.py | 9 +- 6 files changed, 237 insertions(+), 56 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 08461bcdd..957a201aa 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,10 +1,11 @@ +import csv from datetime import date import logging import copy from django import forms from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce -from django.http import HttpResponseRedirect +from django.http import HttpResponse, HttpResponseRedirect from registrar.models.federal_agency import FederalAgency from registrar.utility.admin_helpers import ( get_action_needed_reason_default_email, @@ -42,7 +43,7 @@ from django.contrib.auth.forms import UserChangeForm, UsernameField from django.contrib.admin.views.main import IGNORED_PARAMS from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter -from import_export import resources +from import_export import resources, fields from import_export.admin import ImportExportModelAdmin from django.core.exceptions import ObjectDoesNotExist from django.contrib.admin.widgets import FilteredSelectMultiple @@ -1453,6 +1454,57 @@ class DomainInformationResource(resources.ModelResource): class Meta: model = models.DomainInformation + # Override exports for these columns in DomainInformation to use converted values. These values + # come from @Property functions, which are not automatically included in the export and which we + # want to use in place of the native fields. + organization_name = fields.Field(attribute='converted_organization_name', column_name='organization_name') + generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic_org_type') + federal_type = fields.Field(attribute='converted_federal_type', column_name='federal_type') + federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal_agency') + senior_official = fields.Field(attribute='converted_senior_official', column_name='senior_official') + address_line1 = fields.Field(attribute='converted_address_line1', column_name='address_line1') + address_line2 = fields.Field(attribute='converted_address_line2', column_name='address_line2') + city = fields.Field(attribute='converted_city', column_name='city') + state_territory = fields.Field(attribute='converted_state_territory', column_name='state_territory') + zipcode = fields.Field(attribute='converted_zipcode', column_name='zipcode') + urbanization = fields.Field(attribute='converted_urbanization', column_name='urbanization') + + # Custom getters for the above columns that map to @property functions instead of fields + def dehydrate_organization_name(self, obj): + return obj.converted_organization_name + + def dehydrate_generic_org_type(self, obj): + return obj.converted_generic_org_type + + def dehydrate_federal_type(self, obj): + return obj.converted_federal_type + + def dehydrate_federal_agency(self, obj): + return obj.converted_federal_agency + + def dehydrate_senior_official(self, obj): + return obj.converted_senior_official + + def dehydrate_address_line1(self, obj): + return obj.converted_address_line1 + + def dehydrate_address_line2(self, obj): + return obj.converted_address_line2 + + def dehydrate_city(self, obj): + return obj.converted_city + + def dehydrate_state_territory(self, obj): + return obj.converted_state_territory + + def dehydrate_zipcode(self, obj): + return obj.converted_zipcode + + def dehydrate_urbanization(self, obj): + return obj.converted_urbanization + + + class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Customize domain information admin class.""" @@ -1654,6 +1706,56 @@ class DomainRequestResource(FsmModelResource): class Meta: model = models.DomainRequest + # Override exports for these columns in DomainInformation to use converted values. These values + # come from @Property functions, which are not automatically included in the export and which we + # want to use in place of the native fields. + organization_name = fields.Field(attribute='converted_organization_name', column_name='GEN organization_name') + generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='GEN generic_org_type') + federal_type = fields.Field(attribute='converted_federal_type', column_name='GEN federal_type') + federal_agency = fields.Field(attribute='converted_federal_agency', column_name='GEN federal_agency') + senior_official = fields.Field(attribute='converted_senior_official', column_name='GEN senior_official') + address_line1 = fields.Field(attribute='converted_address_line1', column_name='GEN address_line1') + address_line2 = fields.Field(attribute='converted_address_line2', column_name='GEN address_line2') + city = fields.Field(attribute='converted_city', column_name='GEN city') + state_territory = fields.Field(attribute='converted_state_territory', column_name='GEN state_territory') + zipcode = fields.Field(attribute='converted_zipcode', column_name='GEN zipcode') + urbanization = fields.Field(attribute='converted_urbanization', column_name='GEN urbanization') + senior_official = fields.Field(attribute='converted_urbanization', column_name='GEN senior official') + + # Custom getters for the above columns that map to @property functions instead of fields + def dehydrate_organization_name(self, obj): + return obj.converted_organization_name + + def dehydrate_generic_org_type(self, obj): + return obj.converted_generic_org_type + + def dehydrate_federal_type(self, obj): + return obj.converted_federal_type + + def dehydrate_federal_agency(self, obj): + return obj.converted_federal_agency + + def dehydrate_senior_official(self, obj): + return obj.converted_senior_official + + def dehydrate_address_line1(self, obj): + return obj.converted_address_line1 + + def dehydrate_address_line2(self, obj): + return obj.converted_address_line2 + + def dehydrate_city(self, obj): + return obj.converted_city + + def dehydrate_state_territory(self, obj): + return obj.converted_state_territory + + def dehydrate_zipcode(self, obj): + return obj.converted_zipcode + + def dehydrate_urbanization(self, obj): + return obj.converted_urbanization + class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain requests admin class.""" @@ -2577,7 +2679,48 @@ class DomainResource(FsmModelResource): class Meta: model = models.Domain + #Override the default export so that it matches what is displayed in the admin table for Domains + fields = ( + "name", + "converted_generic_org_type", + "federal_type", + "converted_federal_type", + "converted_federal_agency", + "converted_organization_name", + "custom_election_board", + "converted_city", + "converted_state_territory", + "state", + "expiration_date", + "created_at", + "first_ready", + "deleted", + ) + # Custom getters to retrieve the values from @Proprerty methods in DomainInfo + converted_generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='Converted generic org type') + converted_federal_agency = fields.Field(attribute='converted_federal_agency', column_name='Converted federal agency') + converted_organization_name = fields.Field(attribute='converted_organization_name', column_name='Converted organization name') + converted_city = fields.Field(attribute='converted_city', column_name='city') + converted_state_territory = fields.Field(attribute='converted_state_territory', column_name='Converted state territory') + + # def dehydrate_generic_org_type(self, obj): + # return obj.domain_info.converted_federal_type + + def dehydrate_converted_generic_org_type(self, obj): + return obj.domain_info.converted_generic_org_type + + def dehydrate_converted_federal_agency(self, obj): + return obj.domain_info.converted_federal_agency + + def dehydrate_converted_organization_name(self, obj): + return obj.domain_info.converted_organization_name + + def dehydrate_converted_city(self, obj): + return obj.domain_info.converted_city + + def dehydrate_converted_state_territory(self, obj): + return obj.domain_info.converted_state_territory class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain admin class to add extra buttons.""" @@ -3073,8 +3216,6 @@ def has_change_permission(self, request, obj=None): return True return super().has_change_permission(request, obj) - - class DraftDomainResource(resources.ModelResource): """defines how each field in the referenced model should be mapped to the corresponding fields in the diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index d289eaf90..612dcbf77 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -226,11 +226,6 @@ ExportDataTypeRequests.as_view(), name="export_data_type_requests", ), - path( - "reports/export_data_type_requests/", - ExportDataTypeRequests.as_view(), - name="export_data_type_requests", - ), path( "domain-request//edit/", views.DomainRequestWizard.as_view(), diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 525d7998e..7595eb4f0 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -431,8 +431,8 @@ def get_state_display_of_domain(self): @property def converted_organization_name(self): if self.portfolio: - return self.portfolio.organization_name - return self.organization_name + return "portoflio name" #self.portfolio.organization_name + return "self name" #self.organization_name @property def converted_generic_org_type(self): @@ -495,7 +495,3 @@ def converted_urbanization(self): return self.urbanization - - - - diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 0d8bbd5cf..c62a939a3 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1416,8 +1416,8 @@ def _form_complete(self, request): @property def converted_organization_name(self): if self.portfolio: - return self.portfolio.organization_name - return self.organization_name + return "portfolio name" #self.portfolio.organization_name + return "self name" #self.organization_name @property def converted_generic_org_type(self): @@ -1448,9 +1448,15 @@ def converted_state_territory(self): if self.portfolio: return self.portfolio.state_territory return self.state_territory + + @property + def converted_urbanization(self): + if self.portfolio: + return self.portfolio.urbanization + return self.urbanization @property def converted_senior_official(self): if self.portfolio: return self.portfolio.senior_official - return self.senior_official + return self.senior_official \ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 64d960337..0badcc7ea 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -21,6 +21,11 @@ from registrar.utility.enums import DefaultEmail + +# ---Logger +import logging +from venv import logger +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper logger = logging.getLogger(__name__) @@ -197,6 +202,8 @@ def export_data_to_csv(cls, csv_file, **export_kwargs): All domain metadata: Exports domains of all statuses plus domain managers. """ + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Exporting data") + writer = csv.writer(csv_file) columns = cls.get_columns() sort_fields = cls.get_sort_fields() @@ -226,6 +233,8 @@ def export_data_to_csv(cls, csv_file, **export_kwargs): ) models_dict = convert_queryset_to_dict(annotated_queryset, is_model=False) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"COLUMNS: {columns}") + # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) @@ -374,8 +383,8 @@ def parse_row(cls, columns, model): if first_ready_on is None: first_ready_on = "(blank)" - # organization_type has generic_org_type AND is_election - domain_org_type = model.get("organization_type") + # organization_type has organization_type AND is_election + domain_org_type = model.get("converted_generic_org_type") or model.get("organization_type") human_readable_domain_org_type = DomainRequest.OrgChoicesElectionOffice.get_org_label(domain_org_type) domain_federal_type = model.get("federal_type") human_readable_domain_federal_type = BranchChoices.get_branch_label(domain_federal_type) @@ -392,6 +401,7 @@ def parse_row(cls, columns, model): ): security_contact_email = "(blank)" + # create a dictionary of fields which can be included in output. # "extra_fields" are precomputed fields (generated in the DB or parsed). FIELDS = { @@ -400,12 +410,12 @@ def parse_row(cls, columns, model): "First ready on": first_ready_on, "Expiration date": expiration_date, "Domain type": domain_type, - "Agency": model.get("federal_agency__agency"), - "Organization name": model.get("organization_name"), - "City": model.get("city"), - "State": model.get("state_territory"), + "Agency": model.get("converted_federal_agency__agency"), + "Organization name": model.get("converted_organization_name"), + "City": model.get("converted_city"), + "State": model.get("converted_state_territory"), "SO": model.get("so_name"), - "SO email": model.get("senior_official__email"), + "SO email": model.get("converted_senior_official__email"), "Security contact email": security_contact_email, "Created at": model.get("domain__created_at"), "Deleted": model.get("domain__deleted"), @@ -414,8 +424,20 @@ def parse_row(cls, columns, model): } row = [FIELDS.get(column, "") for column in columns] + + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"PARSING ROW: {row}") + return row + def get_filtered_domain_infos_by_org(domain_infos_to_filter, org_to_filter_by): + """Returns a list of Domain Requests that has been filtered by the given organization value.""" + return domain_infos_to_filter.filter( + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domainInfos.id for domainInfos in domain_infos_to_filter if domainInfos.converted_generic_org_type and domainInfos.converted_generic_org_type == org_to_filter_by + ] + ) + @classmethod def get_sliced_domains(cls, filter_condition): """Get filtered domains counts sliced by org type and election office. @@ -423,23 +445,23 @@ def get_sliced_domains(cls, filter_condition): when a domain has more that one manager. """ - domains = DomainInformation.objects.all().filter(**filter_condition).distinct() - domains_count = domains.count() - federal = domains.filter(generic_org_type=DomainRequest.OrganizationChoices.FEDERAL).distinct().count() - interstate = domains.filter(generic_org_type=DomainRequest.OrganizationChoices.INTERSTATE).count() + domain_informations = DomainInformation.objects.all().filter(**filter_condition).distinct() + domains_count = domain_informations.count() + federal = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.FEDERAL).distinct().count() + interstate = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.INTERSTATE).count() state_or_territory = ( - domains.filter(generic_org_type=DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() ) - tribal = domains.filter(generic_org_type=DomainRequest.OrganizationChoices.TRIBAL).distinct().count() - county = domains.filter(generic_org_type=DomainRequest.OrganizationChoices.COUNTY).distinct().count() - city = domains.filter(generic_org_type=DomainRequest.OrganizationChoices.CITY).distinct().count() + tribal = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.TRIBAL).distinct().count() + county = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.COUNTY).distinct().count() + city = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.CITY).distinct().count() special_district = ( - domains.filter(generic_org_type=DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() ) school_district = ( - domains.filter(generic_org_type=DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() ) - election_board = domains.filter(is_election_board=True).distinct().count() + election_board = domain_informations.filter(is_election_board=True).distinct().count() return [ domains_count, @@ -461,11 +483,15 @@ class DomainDataType(DomainExport): Inherits from BaseExport -> DomainExport """ + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"DomainDataType!!") + @classmethod def get_columns(cls): """ Overrides the columns for CSV export specific to DomainExport. """ + + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"...getting columns") return [ "Domain name", "Status", @@ -524,7 +550,7 @@ def get_select_related(cls): """ Get a list of tables to pass to select_related when building queryset. """ - return ["domain", "senior_official"] + return ["domain", "converted_senior_official"] @classmethod def get_prefetch_related(cls): @@ -660,11 +686,11 @@ def exporting_dr_data_to_csv(cls, response, request=None): cls.safe_get(getattr(request, "all_alternative_domains", None)), cls.safe_get(getattr(request, "all_other_contacts", None)), cls.safe_get(getattr(request, "all_current_websites", None)), - cls.safe_get(getattr(request, "converted_federal_agency", None)), - cls.safe_get(getattr(request.converted_senior_official, "first_name", None)), - cls.safe_get(getattr(request.converted_senior_official, "last_name", None)), - cls.safe_get(getattr(request.converted_senior_official, "email", None)), - cls.safe_get(getattr(request.converted_senior_official, "title", None)), + cls.safe_get(getattr(request, "federal_agency", None)), + cls.safe_get(getattr(request.senior_official, "first_name", None)), + cls.safe_get(getattr(request.senior_official, "last_name", None)), + cls.safe_get(getattr(request.senior_official, "email", None)), + cls.safe_get(getattr(request.senior_official, "title", None)), cls.safe_get(getattr(request.creator, "first_name", None)), cls.safe_get(getattr(request.creator, "last_name", None)), cls.safe_get(getattr(request.creator, "email", None)), @@ -1223,25 +1249,35 @@ class DomainRequestExport(BaseExport): def model(cls): # Return the model class that this export handles return DomainRequest - + + def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter_by): + """Returns a list of Domain Requests that has been filtered by the given organization value""" + return domain_requests_to_filter.filter( + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domainRequest.id for domainRequest in domain_requests_to_filter if domainRequest.converted_generic_org_type and domainRequest.converted_generic_org_type == org_to_filter_by + ] + ) + + @classmethod def get_sliced_requests(cls, filter_condition): """Get filtered requests counts sliced by org type and election office.""" requests = DomainRequest.objects.all().filter(**filter_condition).distinct() requests_count = requests.count() - federal = requests.filter(generic_org_type=DomainRequest.OrganizationChoices.FEDERAL).distinct().count() - interstate = requests.filter(generic_org_type=DomainRequest.OrganizationChoices.INTERSTATE).distinct().count() + federal = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.FEDERAL).distinct().count() + interstate = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.INTERSTATE).distinct().count() state_or_territory = ( - requests.filter(generic_org_type=DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() ) - tribal = requests.filter(generic_org_type=DomainRequest.OrganizationChoices.TRIBAL).distinct().count() - county = requests.filter(generic_org_type=DomainRequest.OrganizationChoices.COUNTY).distinct().count() - city = requests.filter(generic_org_type=DomainRequest.OrganizationChoices.CITY).distinct().count() + tribal = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.TRIBAL).distinct().count() + county = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.COUNTY).distinct().count() + city = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.CITY).distinct().count() special_district = ( - requests.filter(generic_org_type=DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() ) school_district = ( - requests.filter(generic_org_type=DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() ) election_board = requests.filter(is_election_board=True).distinct().count() @@ -1269,7 +1305,7 @@ def parse_row(cls, columns, model): human_readable_federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else None # Handle the org_type field - org_type = model.get("generic_org_type") or model.get("organization_type") + org_type = model.get("converted_generic_org_type") or model.get("organization_type") human_readable_org_type = DomainRequest.OrganizationChoices.get_org_label(org_type) if org_type else None # Handle the status field. Defaults to the wrong format. @@ -1327,9 +1363,9 @@ def parse_row(cls, columns, model): "Creator email": model.get("creator__email"), "Investigator": model.get("investigator__email"), # Untouched fields - "Organization name": model.get("organization_name"), - "City": model.get("city"), - "State/territory": model.get("state_territory"), + "Organization name": model.get("converted_organization_name"), + "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"), diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index d9c4d192c..d34d66daa 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -13,8 +13,12 @@ import logging -logger = logging.getLogger(__name__) +# ---Logger +import logging +from venv import logger +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +logger = logging.getLogger(__name__) class AnalyticsView(View): def get(self, request): @@ -161,7 +165,10 @@ def get(self, request, *args, **kwargs): class ExportDataTypeUser(View): """Returns a domain report for a given user on the request""" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"ExportDataTypeUser") + def get(self, request, *args, **kwargs): + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"ExportDataTypeUser -- get") # match the CSV example with all the fields response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = 'attachment; filename="your-domains.csv"' From ba06660df027db5461bf180fdd45a4a1b0c55ad5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 13 Nov 2024 01:17:28 -0700 Subject: [PATCH 07/29] Remove logs --- src/registrar/utility/csv_export.py | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 0badcc7ea..3646ba894 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -20,12 +20,6 @@ from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail - - -# ---Logger -import logging -from venv import logger -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper logger = logging.getLogger(__name__) @@ -202,7 +196,6 @@ def export_data_to_csv(cls, csv_file, **export_kwargs): All domain metadata: Exports domains of all statuses plus domain managers. """ - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Exporting data") writer = csv.writer(csv_file) columns = cls.get_columns() @@ -233,8 +226,6 @@ def export_data_to_csv(cls, csv_file, **export_kwargs): ) models_dict = convert_queryset_to_dict(annotated_queryset, is_model=False) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"COLUMNS: {columns}") - # Write to csv file before the write_csv cls.write_csv_before(writer, **export_kwargs) @@ -424,8 +415,7 @@ def parse_row(cls, columns, model): } row = [FIELDS.get(column, "") for column in columns] - - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"PARSING ROW: {row}") + return row @@ -483,15 +473,12 @@ class DomainDataType(DomainExport): Inherits from BaseExport -> DomainExport """ - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"DomainDataType!!") - @classmethod def get_columns(cls): """ Overrides the columns for CSV export specific to DomainExport. """ - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, f"...getting columns") return [ "Domain name", "Status", From 503d214ca256fc1c555daa51ed8daa3044747ec5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 22:01:02 -0700 Subject: [PATCH 08/29] Cleanup --- src/registrar/admin.py | 66 +++++++++++----------- src/registrar/models/domain_information.py | 4 +- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 957a201aa..db2943078 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1543,7 +1543,7 @@ def queryset(self, request, queryset): form = DomainInformationAdminForm # Customize column header text - @admin.display(description=_("Converted Generic Org Type")) + @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): return obj.converted_generic_org_type @@ -1709,18 +1709,18 @@ class Meta: # Override exports for these columns in DomainInformation to use converted values. These values # come from @Property functions, which are not automatically included in the export and which we # want to use in place of the native fields. - organization_name = fields.Field(attribute='converted_organization_name', column_name='GEN organization_name') - generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='GEN generic_org_type') - federal_type = fields.Field(attribute='converted_federal_type', column_name='GEN federal_type') - federal_agency = fields.Field(attribute='converted_federal_agency', column_name='GEN federal_agency') - senior_official = fields.Field(attribute='converted_senior_official', column_name='GEN senior_official') - address_line1 = fields.Field(attribute='converted_address_line1', column_name='GEN address_line1') - address_line2 = fields.Field(attribute='converted_address_line2', column_name='GEN address_line2') - city = fields.Field(attribute='converted_city', column_name='GEN city') - state_territory = fields.Field(attribute='converted_state_territory', column_name='GEN state_territory') - zipcode = fields.Field(attribute='converted_zipcode', column_name='GEN zipcode') - urbanization = fields.Field(attribute='converted_urbanization', column_name='GEN urbanization') - senior_official = fields.Field(attribute='converted_urbanization', column_name='GEN senior official') + organization_name = fields.Field(attribute='converted_organization_name', column_name='organization_name') + generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic_org_type') + federal_type = fields.Field(attribute='converted_federal_type', column_name='federal_type') + federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal_agency') + senior_official = fields.Field(attribute='converted_senior_official', column_name='senior_official') + address_line1 = fields.Field(attribute='converted_address_line1', column_name='address_line1') + address_line2 = fields.Field(attribute='converted_address_line2', column_name='address_line2') + city = fields.Field(attribute='converted_city', column_name='city') + state_territory = fields.Field(attribute='converted_state_territory', column_name='state_territory') + zipcode = fields.Field(attribute='converted_zipcode', column_name='zipcode') + urbanization = fields.Field(attribute='converted_urbanization', column_name='urbanization') + senior_official = fields.Field(attribute='converted_urbanization', column_name='senior official') # Custom getters for the above columns that map to @property functions instead of fields def dehydrate_organization_name(self, obj): @@ -2698,14 +2698,11 @@ class Meta: ) # Custom getters to retrieve the values from @Proprerty methods in DomainInfo - converted_generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='Converted generic org type') - converted_federal_agency = fields.Field(attribute='converted_federal_agency', column_name='Converted federal agency') - converted_organization_name = fields.Field(attribute='converted_organization_name', column_name='Converted organization name') + converted_generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic org type') + converted_federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal agency') + converted_organization_name = fields.Field(attribute='converted_organization_name', column_name='organization name') converted_city = fields.Field(attribute='converted_city', column_name='city') - converted_state_territory = fields.Field(attribute='converted_state_territory', column_name='Converted state territory') - - # def dehydrate_generic_org_type(self, obj): - # return obj.domain_info.converted_federal_type + converted_state_territory = fields.Field(attribute='converted_state_territory', column_name='state territory') def dehydrate_converted_generic_org_type(self, obj): return obj.domain_info.converted_generic_org_type @@ -2852,67 +2849,72 @@ def get_queryset(self, request): # ------- Domain Information Fields # --- Generic Org Type - @admin.display(description=_("Converted Generic Org Type")) + # Use converted value in the table + @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): return obj.domain_info.converted_generic_org_type converted_generic_org_type.admin_order_field = "domain_info__converted_generic_org_type" # type: ignore + # Use native value for the change form def generic_org_type(self, obj): return obj.domain_info.get_generic_org_type_display() - # generic_org_type.admin_order_field = "domain_info__generic_org_type" # type: ignore # --- Federal Agency - @admin.display(description=_("Converted Federal Agency")) + @admin.display(description=_("Federal Agency")) def converted_federal_agency(self, obj): return obj.domain_info.converted_federal_agency converted_federal_agency.admin_order_field = "domain_info__converted_federal_agency" # type: ignore + # Use native value for the change form def federal_agency(self, obj): if obj.domain_info: return obj.domain_info.federal_agency else: return None - # federal_agency.admin_order_field = "domain_info__federal_agency" # type: ignore # --- Federal Type - @admin.display(description=_("Converted Federal Type")) + # Use converted value in the table + @admin.display(description=_("Federal Type")) def converted_federal_type(self, obj): return obj.domain_info.converted_federal_type converted_federal_type.admin_order_field = "domain_info__converted_federal_type" # type: ignore + # Use native value for the change form def federal_type(self, obj): return obj.domain_info.federal_type if obj.domain_info else None - # federal_type.admin_order_field = "domain_info__federal_type" # type: ignore # --- Organization Name - @admin.display(description=_("Converted Organization Name")) + # Use converted value in the table + @admin.display(description=_("Organization Name")) def converted_organization_name(self, obj): return obj.domain_info.converted_organization_name converted_organization_name.admin_order_field = "domain_info__converted_organization_name" # type: ignore + # Use native value for the change form def organization_name(self, obj): return obj.domain_info.organization_name if obj.domain_info else None - # organization_name.admin_order_field = "domain_info__organization_name" # type: ignore # --- City - @admin.display(description=_("Converted City")) + # Use converted value in the table + @admin.display(description=_("City")) def converted_city(self, obj): return obj.domain_info.converted_city converted_city.admin_order_field = "domain_info__converted_city" # type: ignore + # Use native value for the change form def city(self, obj): return obj.domain_info.city if obj.domain_info else None - # city.admin_order_field = "domain_info__city" # type: ignore # --- State - @admin.display(description=_("Converted State / territory")) + # Use converted value in the table + @admin.display(description=_("State / territory")) def converted_state_territory(self, obj): return obj.domain_info.converted_state_territory converted_state_territory.admin_order_field = "domain_info__converted_state_territory" # type: ignore + # Use native value for the change form def state_territory(self, obj): return obj.domain_info.state_territory if obj.domain_info else None - # state_territory.admin_order_field = "domain_info__state_territory" # type: ignore def dnssecdata(self, obj): diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 7595eb4f0..bc8f3d9ae 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -431,8 +431,8 @@ def get_state_display_of_domain(self): @property def converted_organization_name(self): if self.portfolio: - return "portoflio name" #self.portfolio.organization_name - return "self name" #self.organization_name + return self.portfolio.organization_name + return self.organization_name @property def converted_generic_org_type(self): From 76ac7a002dbae5356622018a2d5a5f316a0bacc4 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 22:01:15 -0700 Subject: [PATCH 09/29] Added missing converted fields to domain request --- src/registrar/models/domain_request.py | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c62a939a3..de6d9e15f 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1416,8 +1416,8 @@ def _form_complete(self, request): @property def converted_organization_name(self): if self.portfolio: - return "portfolio name" #self.portfolio.organization_name - return "self name" #self.organization_name + return self.portfolio.organization_name + return self.organization_name @property def converted_generic_org_type(self): @@ -1437,6 +1437,18 @@ def converted_federal_type(self): return self.portfolio.federal_type return self.federal_type + @property + def converted_address_line1(self): + if self.portfolio: + return self.portfolio.address_line1 + return self.address_line1 + + @property + def converted_address_line2(self): + if self.portfolio: + return self.portfolio.address_line2 + return self.address_line2 + @property def converted_city(self): if self.portfolio: @@ -1455,6 +1467,12 @@ def converted_urbanization(self): return self.portfolio.urbanization return self.urbanization + @property + def converted_zipcode(self): + if self.portfolio: + return self.portfolio.zipcode + return self.zipcode + @property def converted_senior_official(self): if self.portfolio: From 88554e0e75709be59757d799b5961f79a96cef37 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 22:01:35 -0700 Subject: [PATCH 10/29] Updates to analytics exports --- src/registrar/utility/csv_export.py | 257 +++++++++++++++++++++------- 1 file changed, 196 insertions(+), 61 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 3646ba894..7b84700f2 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -11,15 +11,17 @@ PublicContact, UserDomainRole, ) -from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When +from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When, ExpressionWrapper from django.utils import timezone from django.db.models.functions import Concat, Coalesce from django.contrib.postgres.aggregates import StringAgg +from registrar.models.user import User from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices from registrar.utility.enums import DefaultEmail + logger = logging.getLogger(__name__) @@ -284,6 +286,91 @@ class DomainExport(BaseExport): def model(cls): # Return the model class that this export handles return DomainInformation + + @classmethod + def get_computed_fields(cls, delimiter=", "): + """ + Get a dict of computed fields. + """ + # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # This is for performance purposes. Since we are working with dictionary values and not + # model objects as we export data, trying to reinstate model objects in order to grab @property + # values negatively impacts performance. Therefore, we will follow best practice and use annotations + return { + "converted_federal_agency" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__federal_agency")), + # Otherwise, return the natively assigned value + default=F("federal_agency"), + output_field=CharField(), + ), + "converted_organization_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__organization_name")), + # Otherwise, return the natively assigned value + 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_so_email" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__email")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__email"), + output_field=CharField(), + ), + "converted_senior_official_last_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__last_name")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__last_name"), + output_field=CharField(), + ), + "converted_senior_official_first_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__first_name")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__first_name"), + output_field=CharField(), + ), + "converted_senior_official_title" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__title")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__title"), + output_field=CharField(), + ), + "converted_so_name": Case( + # When portfolio is present, use that senior official instead + When(portfolio__isnull=False, then=Concat( + Coalesce(F("portfolio__senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("portfolio__senior_official__last_name"), Value("")), + output_field=CharField(), + )), + # Otherwise, return the natively assigned senior official + default=Concat( + Coalesce(F("senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("senior_official__last_name"), Value("")), + output_field=CharField(), + ), + output_field=CharField(), + ), + } @classmethod def update_queryset(cls, queryset, **kwargs): @@ -401,12 +488,12 @@ def parse_row(cls, columns, model): "First ready on": first_ready_on, "Expiration date": expiration_date, "Domain type": domain_type, - "Agency": model.get("converted_federal_agency__agency"), + "Agency": model.get("converted_federal_agency"), "Organization name": model.get("converted_organization_name"), "City": model.get("converted_city"), "State": model.get("converted_state_territory"), - "SO": model.get("so_name"), - "SO email": model.get("converted_senior_official__email"), + "SO": model.get("converted_so_name"), + "SO email": model.get("converted_so_email"), "Security contact email": security_contact_email, "Created at": model.get("domain__created_at"), "Deleted": model.get("domain__deleted"), @@ -415,7 +502,6 @@ def parse_row(cls, columns, model): } row = [FIELDS.get(column, "") for column in columns] - return row @@ -537,7 +623,7 @@ def get_select_related(cls): """ Get a list of tables to pass to select_related when building queryset. """ - return ["domain", "converted_senior_official"] + return ["domain", "senior_official"] @classmethod def get_prefetch_related(cls): @@ -546,19 +632,6 @@ def get_prefetch_related(cls): """ return ["permissions"] - @classmethod - def get_computed_fields(cls, delimiter=", "): - """ - Get a dict of computed fields. - """ - return { - "so_name": Concat( - Coalesce(F("senior_official__first_name"), Value("")), - Value(" "), - Coalesce(F("senior_official__last_name"), Value("")), - output_field=CharField(), - ), - } @classmethod def get_related_table_fields(cls): @@ -673,11 +746,11 @@ def exporting_dr_data_to_csv(cls, response, request=None): cls.safe_get(getattr(request, "all_alternative_domains", None)), cls.safe_get(getattr(request, "all_other_contacts", None)), cls.safe_get(getattr(request, "all_current_websites", None)), - cls.safe_get(getattr(request, "federal_agency", None)), - cls.safe_get(getattr(request.senior_official, "first_name", None)), - cls.safe_get(getattr(request.senior_official, "last_name", None)), - cls.safe_get(getattr(request.senior_official, "email", None)), - cls.safe_get(getattr(request.senior_official, "title", None)), + cls.safe_get(getattr(request, "converted_federal_agency", None)), + cls.safe_get(getattr(request.converted_senior_official, "first_name", None)), + cls.safe_get(getattr(request.converted_senior_official, "last_name", None)), + cls.safe_get(getattr(request.converted_senior_official, "email", None)), + cls.safe_get(getattr(request.converted_senior_official, "title", None)), cls.safe_get(getattr(request.creator, "first_name", None)), cls.safe_get(getattr(request.creator, "last_name", None)), cls.safe_get(getattr(request.creator, "email", None)), @@ -763,20 +836,6 @@ def get_filter_conditions(cls): ], ) - @classmethod - def get_computed_fields(cls, delimiter=", "): - """ - Get a dict of computed fields. - """ - return { - "so_name": Concat( - Coalesce(F("senior_official__first_name"), Value("")), - Value(" "), - Coalesce(F("senior_official__last_name"), Value("")), - output_field=CharField(), - ), - } - @classmethod def get_related_table_fields(cls): """ @@ -858,20 +917,6 @@ def get_filter_conditions(cls): ], ) - @classmethod - def get_computed_fields(cls, delimiter=", "): - """ - Get a dict of computed fields. - """ - return { - "so_name": Concat( - Coalesce(F("senior_official__first_name"), Value("")), - Value(" "), - Coalesce(F("senior_official__last_name"), Value("")), - output_field=CharField(), - ), - } - @classmethod def get_related_table_fields(cls): """ @@ -1246,6 +1291,91 @@ def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter ] ) + + @classmethod + def get_computed_fields(cls, delimiter=", "): + """ + Get a dict of computed fields. + """ + # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # This is for performance purposes. Since we are working with dictionary values and not + # model objects as we export data, trying to reinstate model objects in order to grab @property + # values negatively impacts performance. Therefore, we will follow best practice and use annotations + return { + "converted_federal_agency" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__federal_agency")), + # Otherwise, return the natively assigned value + default=F("federal_agency"), + output_field=CharField(), + ), + "converted_organization_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__organization_name")), + # Otherwise, return the natively assigned value + 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_so_email" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__email")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__email"), + output_field=CharField(), + ), + "converted_senior_official_last_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__last_name")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__last_name"), + output_field=CharField(), + ), + "converted_senior_official_first_name" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__first_name")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__first_name"), + output_field=CharField(), + ), + "converted_senior_official_title" : Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__senior_official__title")), + # Otherwise, return the natively assigned senior official + default=F("senior_official__title"), + output_field=CharField(), + ), + "converted_so_name": Case( + # When portfolio is present, use that senior official instead + When(portfolio__isnull=False, then=Concat( + Coalesce(F("portfolio__senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("portfolio__senior_official__last_name"), Value("")), + output_field=CharField(), + )), + # Otherwise, return the natively assigned senior official + default=Concat( + Coalesce(F("senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("senior_official__last_name"), Value("")), + output_field=CharField(), + ), + output_field=CharField(), + ), + } @classmethod def get_sliced_requests(cls, filter_condition): @@ -1340,11 +1470,11 @@ def parse_row(cls, columns, model): "Other contacts": model.get("all_other_contacts"), "Current websites": model.get("all_current_websites"), # Untouched FK fields - passed into the request dict. - "Federal agency": model.get("federal_agency__agency"), - "SO first name": model.get("senior_official__first_name"), - "SO last name": model.get("senior_official__last_name"), - "SO email": model.get("senior_official__email"), - "SO title/role": model.get("senior_official__title"), + "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"), + "SO email": model.get("converted_so_email"), + "SO title/role": model.get("converted_senior_official_title"), "Creator first name": model.get("creator__first_name"), "Creator last name": model.get("creator__last_name"), "Creator email": model.get("creator__email"), @@ -1411,8 +1541,7 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return ["requested_domain__name"] - - + class DomainRequestDataFull(DomainRequestExport): """ Shows all but STARTED requests @@ -1492,7 +1621,11 @@ def get_computed_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ - return { + # Get computed fields from the parent class + computed_fields = super().get_computed_fields() + + # Add additional computed fields + computed_fields.update({ "creator_approved_domains_count": cls.get_creator_approved_domains_count_query(), "creator_active_requests_count": cls.get_creator_active_requests_count_query(), "all_current_websites": StringAgg("current_websites__website", delimiter=delimiter, distinct=True), @@ -1509,7 +1642,9 @@ def get_computed_fields(cls, delimiter=", "): delimiter=delimiter, distinct=True, ), - } + }) + + return computed_fields @classmethod def get_related_table_fields(cls): From ca8cfda03d0ece3524d24f1af5bebbe137e0183e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 23:36:30 -0700 Subject: [PATCH 11/29] unit test adjustments --- src/registrar/tests/test_reports.py | 95 +++++++++++++++-------------- 1 file changed, 48 insertions(+), 47 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index ae1b3b1c1..a64ce2cea 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -63,10 +63,10 @@ def test_generate_federal_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), - call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), + call('cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n'), + call('cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n'), + call('adomain10.gov,Federal,8,,,,(blank)\r\n'), + call('ddomain3.gov,Federal,8,,,,(blank)\r\n'), ] # We don't actually want to write anything for a test case, # we just want to verify what is being written. @@ -85,11 +85,11 @@ def test_generate_full_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), - call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), - call("zdomain12.gov,Interstate,,,,,(blank)\r\n"), + call('cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n'), + call('cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n'), + call('adomain10.gov,Federal,8,,,,(blank)\r\n'), + call('ddomain3.gov,Federal,8,,,,(blank)\r\n'), + call('zdomain12.gov,Interstate,,,,,(blank)\r\n'), ] # We don't actually want to write anything for a test case, # we just want to verify what is being written. @@ -245,25 +245,21 @@ def test_domain_data_type(self): expected_content = ( "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO," "SO email,Security contact email,Domain managers,Invited domain managers\n" - "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,World War I Centennial Commission,,,,(blank),,," - "meoward@rocks.com,\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,World War I Centennial Commission,,," - ',,,(blank),"big_lebowski@dude.co, info@example.com, meoward@rocks.com",' - "woofwardthethird@rocks.com\n" - "adomain10.gov,Ready,2024-04-03,(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,," - "squeaker@rocks.com\n" - "bdomain4.gov,Unknown,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "bdomain5.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "bdomain6.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "ddomain3.gov,On hold,(blank),2023-11-15,Federal,Armed Forces Retirement Home,,,,,," - "security@mail.gov,,\n" - "sdomain8.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "xdomain7.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "zdomain9.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,,(blank),,,,\n" - "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,,(blank),,," - "meoward@rocks.com,squeaker@rocks.com\n" - "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,,(blank),,,meoward@rocks.com,\n" + "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,188,,,, ,,(blank),meoward@rocks.com,\n" + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,188,,,, ,,(blank)," + "\"big_lebowski@dude.co, info@example.com, meoward@rocks.com\",woofwardthethird@rocks.com\n" + "adomain10.gov,Ready,2024-04-03,(blank),Federal,189,,,, ,,(blank),,squeaker@rocks.com\n" + "bdomain4.gov,Unknown,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "bdomain5.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "bdomain6.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "ddomain3.gov,On hold,(blank),2023-11-15,Federal,189,,,, ,,security@mail.gov,,\n" + "sdomain8.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "xdomain7.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "zdomain9.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,squeaker@rocks.com\n" + "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,\n" ) + # Normalize line endings and remove commas, # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() @@ -306,17 +302,19 @@ def test_domain_data_type_user(self): "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name," "City,State,SO,SO email," "Security contact email,Domain managers,Invited domain managers\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,World War I Centennial Commission,,,, ,," - '(blank),"big_lebowski@dude.co, info@example.com, meoward@rocks.com",' - "woofwardthethird@rocks.com\n" + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,190,,,, ,,(blank)," + "\"big_lebowski@dude.co, info@example.com, meoward@rocks.com\",woofwardthethird@rocks.com\n" "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank)," - '"info@example.com, meoward@rocks.com",squeaker@rocks.com\n' + "\"info@example.com, meoward@rocks.com\",squeaker@rocks.com\n" ) # Normalize line endings and remove commas, # 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) @@ -485,12 +483,13 @@ def test_domain_data_full(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" - "cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,,,,security@mail.gov\n" + "cdomain11.gov,Federal - Executive,186,,,,(blank)\n" + "defaultsecurity.gov,Federal - Executive,186,,,,(blank)\n" + "adomain10.gov,Federal,187,,,,(blank)\n" + "ddomain3.gov,Federal,187,,,,security@mail.gov\n" "zdomain12.gov,Interstate,,,,,(blank)\n" ) + # Normalize line endings and remove commas, # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() @@ -498,7 +497,7 @@ def test_domain_data_full(self): self.maxDiff = None self.assertEqual(csv_content, expected_content) - @less_console_noise_decorator + # @less_console_noise_decorator def test_domain_data_federal(self): """Shows security contacts, filtered by state and org type""" # Add security email information @@ -525,11 +524,12 @@ def test_domain_data_federal(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" - "cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,,,,security@mail.gov\n" + "cdomain11.gov,Federal - Executive,184,,,,(blank)\n" + "defaultsecurity.gov,Federal - Executive,184,,,,(blank)\n" + "adomain10.gov,Federal,185,,,,(blank)\n" + "ddomain3.gov,Federal,185,,,,security@mail.gov\n" ) + # Normalize line endings and remove commas, # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() @@ -579,13 +579,13 @@ def test_domain_growth(self): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date, Deleted\n" - "cdomain1.gov,Federal-Executive,World War I Centennial Commission,,,,Ready,(blank)\n" - "adomain10.gov,Federal,Armed Forces Retirement Home,,,,Ready,(blank)\n" - "cdomain11.govFederal-ExecutiveWorldWarICentennialCommissionReady(blank)\n" - "zdomain12.govInterstateReady(blank)\n" - "zdomain9.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-01\n" - "sdomain8.gov,Federal,Armed Forces Retirement Home,,,,Deleted,(blank),2024-04-02\n" - "xdomain7.gov,FederalArmedForcesRetirementHome,Deleted,(blank),2024-04-02\n" + "cdomain1.gov,Federal-Executive,194,Ready,(blank)\n" + "adomain10.gov,Federal,195,Ready,(blank)\n" + "cdomain11.gov,Federal-Executive,194,Ready,(blank)\n" + "zdomain12.gov,Interstate,Ready,(blank)\n" + "zdomain9.gov,Federal,195,Deleted,(blank),2024-04-01\n" + "sdomain8.gov,Federal,195,Deleted,(blank),2024-04-02\n" + "xdomain7.gov,Federal,195,Deleted,(blank),2024-04-02\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace @@ -593,6 +593,7 @@ def test_domain_growth(self): csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() ) expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + self.assertEqual(csv_content, expected_content) @less_console_noise_decorator From 34ba850277b46443981f640b48818453790432dd Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 23:42:42 -0700 Subject: [PATCH 12/29] linted --- src/registrar/admin.py | 162 +++++++------- src/registrar/models/domain.py | 2 +- src/registrar/models/domain_information.py | 4 +- src/registrar/models/domain_request.py | 4 +- src/registrar/tests/test_reports.py | 26 +-- src/registrar/utility/csv_export.py | 241 ++++++++++++++------- src/registrar/views/report_views.py | 9 +- 7 files changed, 261 insertions(+), 187 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index db2943078..56b5fa4fe 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1,11 +1,10 @@ -import csv from datetime import date import logging import copy from django import forms from django.db.models import Value, CharField, Q from django.db.models.functions import Concat, Coalesce -from django.http import HttpResponse, HttpResponseRedirect +from django.http import HttpResponseRedirect from registrar.models.federal_agency import FederalAgency from registrar.utility.admin_helpers import ( get_action_needed_reason_default_email, @@ -1457,53 +1456,51 @@ class Meta: # Override exports for these columns in DomainInformation to use converted values. These values # come from @Property functions, which are not automatically included in the export and which we # want to use in place of the native fields. - organization_name = fields.Field(attribute='converted_organization_name', column_name='organization_name') - generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic_org_type') - federal_type = fields.Field(attribute='converted_federal_type', column_name='federal_type') - federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal_agency') - senior_official = fields.Field(attribute='converted_senior_official', column_name='senior_official') - address_line1 = fields.Field(attribute='converted_address_line1', column_name='address_line1') - address_line2 = fields.Field(attribute='converted_address_line2', column_name='address_line2') - city = fields.Field(attribute='converted_city', column_name='city') - state_territory = fields.Field(attribute='converted_state_territory', column_name='state_territory') - zipcode = fields.Field(attribute='converted_zipcode', column_name='zipcode') - urbanization = fields.Field(attribute='converted_urbanization', column_name='urbanization') + organization_name = fields.Field(attribute="converted_organization_name", column_name="organization_name") + generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic_org_type") + federal_type = fields.Field(attribute="converted_federal_type", column_name="federal_type") + federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal_agency") + senior_official = fields.Field(attribute="converted_senior_official", column_name="senior_official") + address_line1 = fields.Field(attribute="converted_address_line1", column_name="address_line1") + address_line2 = fields.Field(attribute="converted_address_line2", column_name="address_line2") + city = fields.Field(attribute="converted_city", column_name="city") + state_territory = fields.Field(attribute="converted_state_territory", column_name="state_territory") + zipcode = fields.Field(attribute="converted_zipcode", column_name="zipcode") + urbanization = fields.Field(attribute="converted_urbanization", column_name="urbanization") # Custom getters for the above columns that map to @property functions instead of fields def dehydrate_organization_name(self, obj): return obj.converted_organization_name - + def dehydrate_generic_org_type(self, obj): return obj.converted_generic_org_type - + def dehydrate_federal_type(self, obj): return obj.converted_federal_type - + def dehydrate_federal_agency(self, obj): return obj.converted_federal_agency - + def dehydrate_senior_official(self, obj): return obj.converted_senior_official - + def dehydrate_address_line1(self, obj): return obj.converted_address_line1 - + def dehydrate_address_line2(self, obj): return obj.converted_address_line2 - + def dehydrate_city(self, obj): return obj.converted_city - + def dehydrate_state_territory(self, obj): return obj.converted_state_territory - + def dehydrate_zipcode(self, obj): return obj.converted_zipcode - + def dehydrate_urbanization(self, obj): return obj.converted_urbanization - - class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): @@ -1515,7 +1512,7 @@ class GenericOrgFilter(admin.SimpleListFilter): organization in the Domain Information object.""" title = "generic organization" - parameter_name = 'converted_generic_orgs' + parameter_name = "converted_generic_orgs" def lookups(self, request, model_admin): converted_generic_orgs = set() @@ -1524,7 +1521,7 @@ def lookups(self, request, model_admin): converted_generic_org = domainInfo.converted_generic_org_type if converted_generic_org: converted_generic_orgs.add(converted_generic_org) - + return sorted((org, org) for org in converted_generic_orgs) # Filter queryset @@ -1533,7 +1530,10 @@ def queryset(self, request, queryset): return queryset.filter( # Filter based on the generic org value returned by converted_generic_org_type id__in=[ - domainInfo.id for domainInfo in queryset if domainInfo.converted_generic_org_type and domainInfo.converted_generic_org_type == self.value() + domainInfo.id + for domainInfo in queryset + if domainInfo.converted_generic_org_type + and domainInfo.converted_generic_org_type == self.value() ] ) return queryset @@ -1709,50 +1709,50 @@ class Meta: # Override exports for these columns in DomainInformation to use converted values. These values # come from @Property functions, which are not automatically included in the export and which we # want to use in place of the native fields. - organization_name = fields.Field(attribute='converted_organization_name', column_name='organization_name') - generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic_org_type') - federal_type = fields.Field(attribute='converted_federal_type', column_name='federal_type') - federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal_agency') - senior_official = fields.Field(attribute='converted_senior_official', column_name='senior_official') - address_line1 = fields.Field(attribute='converted_address_line1', column_name='address_line1') - address_line2 = fields.Field(attribute='converted_address_line2', column_name='address_line2') - city = fields.Field(attribute='converted_city', column_name='city') - state_territory = fields.Field(attribute='converted_state_territory', column_name='state_territory') - zipcode = fields.Field(attribute='converted_zipcode', column_name='zipcode') - urbanization = fields.Field(attribute='converted_urbanization', column_name='urbanization') - senior_official = fields.Field(attribute='converted_urbanization', column_name='senior official') + organization_name = fields.Field(attribute="converted_organization_name", column_name="organization_name") + generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic_org_type") + federal_type = fields.Field(attribute="converted_federal_type", column_name="federal_type") + federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal_agency") + senior_official = fields.Field(attribute="converted_senior_official", column_name="senior_official") + address_line1 = fields.Field(attribute="converted_address_line1", column_name="address_line1") + address_line2 = fields.Field(attribute="converted_address_line2", column_name="address_line2") + city = fields.Field(attribute="converted_city", column_name="city") + state_territory = fields.Field(attribute="converted_state_territory", column_name="state_territory") + zipcode = fields.Field(attribute="converted_zipcode", column_name="zipcode") + urbanization = fields.Field(attribute="converted_urbanization", column_name="urbanization") + senior_official = fields.Field(attribute="converted_urbanization", column_name="senior official") # Custom getters for the above columns that map to @property functions instead of fields def dehydrate_organization_name(self, obj): return obj.converted_organization_name - + def dehydrate_generic_org_type(self, obj): return obj.converted_generic_org_type - + def dehydrate_federal_type(self, obj): return obj.converted_federal_type - + def dehydrate_federal_agency(self, obj): return obj.converted_federal_agency - + def dehydrate_senior_official(self, obj): return obj.converted_senior_official - + def dehydrate_address_line1(self, obj): return obj.converted_address_line1 - + def dehydrate_address_line2(self, obj): return obj.converted_address_line2 - + def dehydrate_city(self, obj): return obj.converted_city - + def dehydrate_state_territory(self, obj): return obj.converted_state_territory - + def dehydrate_zipcode(self, obj): return obj.converted_zipcode - + def dehydrate_urbanization(self, obj): return obj.converted_urbanization @@ -2679,7 +2679,7 @@ class DomainResource(FsmModelResource): class Meta: model = models.Domain - #Override the default export so that it matches what is displayed in the admin table for Domains + # Override the default export so that it matches what is displayed in the admin table for Domains fields = ( "name", "converted_generic_org_type", @@ -2698,27 +2698,28 @@ class Meta: ) # Custom getters to retrieve the values from @Proprerty methods in DomainInfo - converted_generic_org_type = fields.Field(attribute='converted_generic_org_type', column_name='generic org type') - converted_federal_agency = fields.Field(attribute='converted_federal_agency', column_name='federal agency') - converted_organization_name = fields.Field(attribute='converted_organization_name', column_name='organization name') - converted_city = fields.Field(attribute='converted_city', column_name='city') - converted_state_territory = fields.Field(attribute='converted_state_territory', column_name='state territory') - + converted_generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic org type") + converted_federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal agency") + converted_organization_name = fields.Field(attribute="converted_organization_name", column_name="organization name") + converted_city = fields.Field(attribute="converted_city", column_name="city") + converted_state_territory = fields.Field(attribute="converted_state_territory", column_name="state territory") + def dehydrate_converted_generic_org_type(self, obj): return obj.domain_info.converted_generic_org_type - + def dehydrate_converted_federal_agency(self, obj): return obj.domain_info.converted_federal_agency - + def dehydrate_converted_organization_name(self, obj): return obj.domain_info.converted_organization_name - + def dehydrate_converted_city(self, obj): return obj.domain_info.converted_city - + def dehydrate_converted_state_territory(self, obj): return obj.domain_info.converted_state_territory + class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain admin class to add extra buttons.""" @@ -2742,14 +2743,14 @@ def queryset(self, request, queryset): return queryset.filter(domain_info__is_election_board=True) if self.value() == "0": return queryset.filter(Q(domain_info__is_election_board=False) | Q(domain_info__is_election_board=None)) - + class GenericOrgFilter(admin.SimpleListFilter): """Custom Generic Organization filter that accomodates portfolio feature. If we have a portfolio, use the portfolio's organization. If not, use the organization in the Domain Information object.""" title = "generic organization" - parameter_name = 'converted_generic_orgs' + parameter_name = "converted_generic_orgs" def lookups(self, request, model_admin): converted_generic_orgs = set() @@ -2758,7 +2759,7 @@ def lookups(self, request, model_admin): converted_generic_org = domainInfo.converted_generic_org_type if converted_generic_org: converted_generic_orgs.add(converted_generic_org) - + return sorted((org, org) for org in converted_generic_orgs) # Filter queryset @@ -2767,18 +2768,21 @@ def queryset(self, request, queryset): return queryset.filter( # Filter based on the generic org value returned by converted_generic_org_type id__in=[ - domain.id for domain in queryset if domain.domain_info.converted_generic_org_type and domain.domain_info.converted_generic_org_type == self.value() + domain.id + for domain in queryset + if domain.domain_info.converted_generic_org_type + and domain.domain_info.converted_generic_org_type == self.value() ] ) return queryset - + class FederalTypeFilter(admin.SimpleListFilter): """Custom Federal Type filter that accomodates portfolio feature. If we have a portfolio, use the portfolio's federal type. If not, use the federal type in the Domain Information object.""" title = "federal type" - parameter_name = 'converted_federal_types' + parameter_name = "converted_federal_types" def lookups(self, request, model_admin): converted_federal_types = set() @@ -2788,7 +2792,7 @@ def lookups(self, request, model_admin): converted_federal_type = domainInfo.converted_federal_type if converted_federal_type: converted_federal_types.add(converted_federal_type) - + return sorted((fed, fed) for fed in converted_federal_types) # Filter queryset @@ -2797,12 +2801,14 @@ def queryset(self, request, queryset): return queryset.filter( # Filter based on the generic org value returned by converted_generic_org_type id__in=[ - domain.id for domain in queryset if domain.domain_info.converted_federal_type and domain.domain_info.converted_federal_type == self.value() + domain.id + for domain in queryset + if domain.domain_info.converted_federal_type + and domain.domain_info.converted_federal_type == self.value() ] ) return queryset - def get_queryset(self, request): """Custom get_queryset to filter by portfolio if portfolio is in the request params.""" @@ -2813,12 +2819,12 @@ def get_queryset(self, request): # Further filter the queryset by the portfolio qs = qs.filter(domain_info__portfolio=portfolio_id) return qs - + # Filters list_filter = [GenericOrgFilter, FederalTypeFilter, ElectionOfficeFilter, "state"] - + # ------- END FILTERS - + # Inlines inlines = [DomainInformationInline] @@ -2853,8 +2859,9 @@ def get_queryset(self, request): @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): return obj.domain_info.converted_generic_org_type + converted_generic_org_type.admin_order_field = "domain_info__converted_generic_org_type" # type: ignore - + # Use native value for the change form def generic_org_type(self, obj): return obj.domain_info.get_generic_org_type_display() @@ -2863,6 +2870,7 @@ def generic_org_type(self, obj): @admin.display(description=_("Federal Agency")) def converted_federal_agency(self, obj): return obj.domain_info.converted_federal_agency + converted_federal_agency.admin_order_field = "domain_info__converted_federal_agency" # type: ignore # Use native value for the change form @@ -2877,6 +2885,7 @@ def federal_agency(self, obj): @admin.display(description=_("Federal Type")) def converted_federal_type(self, obj): return obj.domain_info.converted_federal_type + converted_federal_type.admin_order_field = "domain_info__converted_federal_type" # type: ignore # Use native value for the change form @@ -2888,6 +2897,7 @@ def federal_type(self, obj): @admin.display(description=_("Organization Name")) def converted_organization_name(self, obj): return obj.domain_info.converted_organization_name + converted_organization_name.admin_order_field = "domain_info__converted_organization_name" # type: ignore # Use native value for the change form @@ -2899,6 +2909,7 @@ def organization_name(self, obj): @admin.display(description=_("City")) def converted_city(self, obj): return obj.domain_info.converted_city + converted_city.admin_order_field = "domain_info__converted_city" # type: ignore # Use native value for the change form @@ -2910,13 +2921,13 @@ def city(self, obj): @admin.display(description=_("State / territory")) def converted_state_territory(self, obj): return obj.domain_info.converted_state_territory + converted_state_territory.admin_order_field = "domain_info__converted_state_territory" # type: ignore # Use native value for the change form def state_territory(self, obj): return obj.domain_info.state_territory if obj.domain_info else None - def dnssecdata(self, obj): return "Yes" if obj.dnssecdata else "No" @@ -2948,7 +2959,6 @@ def custom_election_board(self, obj): custom_election_board.admin_order_field = "domain_info__is_election_board" # type: ignore custom_election_board.short_description = "Election office" # type: ignore - # Search search_fields = ["name"] search_help_text = "Search by domain name." diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 339e4dd20..7fdc56971 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2078,4 +2078,4 @@ def _get_property(self, property): if property in self._cache: return self._cache[property] else: - raise KeyError("Requested key %s was not found in registry cache." % str(property)) \ No newline at end of file + raise KeyError("Requested key %s was not found in registry cache." % str(property)) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index bc8f3d9ae..db5416cc2 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -475,7 +475,7 @@ def converted_city(self): if self.portfolio: return self.portfolio.city return self.city - + @property def converted_state_territory(self): if self.portfolio: @@ -493,5 +493,3 @@ def converted_urbanization(self): if self.portfolio: return self.portfolio.urbanization return self.urbanization - - diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index de6d9e15f..eecc6e3c1 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1460,7 +1460,7 @@ def converted_state_territory(self): if self.portfolio: return self.portfolio.state_territory return self.state_territory - + @property def converted_urbanization(self): if self.portfolio: @@ -1477,4 +1477,4 @@ def converted_zipcode(self): def converted_senior_official(self): if self.portfolio: return self.portfolio.senior_official - return self.senior_official \ No newline at end of file + return self.senior_official diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index a64ce2cea..069042ed9 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -63,10 +63,10 @@ def test_generate_federal_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call('cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n'), - call('cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n'), - call('adomain10.gov,Federal,8,,,,(blank)\r\n'), - call('ddomain3.gov,Federal,8,,,,(blank)\r\n'), + call("cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("adomain10.gov,Federal,8,,,,(blank)\r\n"), + call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), ] # We don't actually want to write anything for a test case, # we just want to verify what is being written. @@ -85,11 +85,11 @@ def test_generate_full_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call('cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n'), - call('cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n'), - call('adomain10.gov,Federal,8,,,,(blank)\r\n'), - call('ddomain3.gov,Federal,8,,,,(blank)\r\n'), - call('zdomain12.gov,Interstate,,,,,(blank)\r\n'), + call("cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("adomain10.gov,Federal,8,,,,(blank)\r\n"), + call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), + call("zdomain12.gov,Interstate,,,,,(blank)\r\n"), ] # We don't actually want to write anything for a test case, # we just want to verify what is being written. @@ -247,7 +247,7 @@ def test_domain_data_type(self): "SO email,Security contact email,Domain managers,Invited domain managers\n" "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,188,,,, ,,(blank),meoward@rocks.com,\n" "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,188,,,, ,,(blank)," - "\"big_lebowski@dude.co, info@example.com, meoward@rocks.com\",woofwardthethird@rocks.com\n" + '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' "adomain10.gov,Ready,2024-04-03,(blank),Federal,189,,,, ,,(blank),,squeaker@rocks.com\n" "bdomain4.gov,Unknown,(blank),(blank),Federal,189,,,, ,,(blank),,\n" "bdomain5.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" @@ -303,9 +303,9 @@ def test_domain_data_type_user(self): "City,State,SO,SO email," "Security contact email,Domain managers,Invited domain managers\n" "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,190,,,, ,,(blank)," - "\"big_lebowski@dude.co, info@example.com, meoward@rocks.com\",woofwardthethird@rocks.com\n" + '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank)," - "\"info@example.com, meoward@rocks.com\",squeaker@rocks.com\n" + '"info@example.com, meoward@rocks.com",squeaker@rocks.com\n' ) # Normalize line endings and remove commas, @@ -313,8 +313,6 @@ def test_domain_data_type_user(self): 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) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 7b84700f2..fff55baed 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -11,11 +11,21 @@ PublicContact, UserDomainRole, ) -from django.db.models import Case, CharField, Count, DateField, F, ManyToManyField, Q, QuerySet, Value, When, ExpressionWrapper +from django.db.models import ( + Case, + CharField, + Count, + DateField, + F, + ManyToManyField, + Q, + QuerySet, + Value, + When, +) from django.utils import timezone from django.db.models.functions import Concat, Coalesce from django.contrib.postgres.aggregates import StringAgg -from registrar.models.user import User from registrar.models.utility.generic_helper import convert_queryset_to_dict from registrar.templatetags.custom_filters import get_region from registrar.utility.constants import BranchChoices @@ -286,67 +296,67 @@ class DomainExport(BaseExport): def model(cls): # Return the model class that this export handles return DomainInformation - + @classmethod def get_computed_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ - # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # NOTE: These computed fields imitate @Property functions in the Domain model where needed. # This is for performance purposes. Since we are working with dictionary values and not # model objects as we export data, trying to reinstate model objects in order to grab @property # values negatively impacts performance. Therefore, we will follow best practice and use annotations return { - "converted_federal_agency" : Case( + "converted_federal_agency": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__federal_agency")), # Otherwise, return the natively assigned value default=F("federal_agency"), output_field=CharField(), ), - "converted_organization_name" : Case( + "converted_organization_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__organization_name")), # Otherwise, return the natively assigned value default=F("organization_name"), output_field=CharField(), ), - "converted_city" : Case( + "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( + "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_so_email" : Case( + "converted_so_email": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__email")), # Otherwise, return the natively assigned senior official default=F("senior_official__email"), output_field=CharField(), ), - "converted_senior_official_last_name" : Case( + "converted_senior_official_last_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__last_name")), # Otherwise, return the natively assigned senior official default=F("senior_official__last_name"), output_field=CharField(), ), - "converted_senior_official_first_name" : Case( + "converted_senior_official_first_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__first_name")), # Otherwise, return the natively assigned senior official default=F("senior_official__first_name"), output_field=CharField(), ), - "converted_senior_official_title" : Case( + "converted_senior_official_title": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__title")), # Otherwise, return the natively assigned senior official @@ -355,12 +365,15 @@ def get_computed_fields(cls, delimiter=", "): ), "converted_so_name": Case( # When portfolio is present, use that senior official instead - When(portfolio__isnull=False, then=Concat( - Coalesce(F("portfolio__senior_official__first_name"), Value("")), - Value(" "), - Coalesce(F("portfolio__senior_official__last_name"), Value("")), - output_field=CharField(), - )), + When( + portfolio__isnull=False, + then=Concat( + Coalesce(F("portfolio__senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("portfolio__senior_official__last_name"), Value("")), + output_field=CharField(), + ), + ), # Otherwise, return the natively assigned senior official default=Concat( Coalesce(F("senior_official__first_name"), Value("")), @@ -479,7 +492,6 @@ def parse_row(cls, columns, model): ): security_contact_email = "(blank)" - # create a dictionary of fields which can be included in output. # "extra_fields" are precomputed fields (generated in the DB or parsed). FIELDS = { @@ -508,12 +520,14 @@ def parse_row(cls, columns, model): def get_filtered_domain_infos_by_org(domain_infos_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value.""" return domain_infos_to_filter.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domainInfos.id for domainInfos in domain_infos_to_filter if domainInfos.converted_generic_org_type and domainInfos.converted_generic_org_type == org_to_filter_by - ] - ) - + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domainInfos.id + for domainInfos in domain_infos_to_filter + if domainInfos.converted_generic_org_type and domainInfos.converted_generic_org_type == org_to_filter_by + ] + ) + @classmethod def get_sliced_domains(cls, filter_condition): """Get filtered domains counts sliced by org type and election office. @@ -523,19 +537,47 @@ def get_sliced_domains(cls, filter_condition): domain_informations = DomainInformation.objects.all().filter(**filter_condition).distinct() domains_count = domain_informations.count() - federal = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.FEDERAL).distinct().count() - interstate = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.INTERSTATE).count() + federal = ( + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.FEDERAL) + .distinct() + .count() + ) + interstate = cls.get_filtered_domain_infos_by_org( + domain_informations, DomainRequest.OrganizationChoices.INTERSTATE + ).count() state_or_territory = ( - cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() + cls.get_filtered_domain_infos_by_org( + domain_informations, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY + ) + .distinct() + .count() + ) + tribal = ( + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.TRIBAL) + .distinct() + .count() + ) + county = ( + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.COUNTY) + .distinct() + .count() + ) + city = ( + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.CITY) + .distinct() + .count() ) - tribal = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.TRIBAL).distinct().count() - county = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.COUNTY).distinct().count() - city = cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.CITY).distinct().count() special_district = ( - cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + cls.get_filtered_domain_infos_by_org( + domain_informations, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT + ) + .distinct() + .count() ) school_district = ( - cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT) + .distinct() + .count() ) election_board = domain_informations.filter(is_election_board=True).distinct().count() @@ -632,7 +674,6 @@ def get_prefetch_related(cls): """ return ["permissions"] - @classmethod def get_related_table_fields(cls): """ @@ -1281,77 +1322,79 @@ class DomainRequestExport(BaseExport): def model(cls): # Return the model class that this export handles return DomainRequest - + def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value""" return domain_requests_to_filter.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domainRequest.id for domainRequest in domain_requests_to_filter if domainRequest.converted_generic_org_type and domainRequest.converted_generic_org_type == org_to_filter_by - ] - ) - + # Filter based on the generic org value returned by converted_generic_org_type + id__in=[ + domainRequest.id + for domainRequest in domain_requests_to_filter + if domainRequest.converted_generic_org_type + and domainRequest.converted_generic_org_type == org_to_filter_by + ] + ) @classmethod def get_computed_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ - # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # NOTE: These computed fields imitate @Property functions in the Domain model where needed. # This is for performance purposes. Since we are working with dictionary values and not # model objects as we export data, trying to reinstate model objects in order to grab @property # values negatively impacts performance. Therefore, we will follow best practice and use annotations return { - "converted_federal_agency" : Case( + "converted_federal_agency": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__federal_agency")), # Otherwise, return the natively assigned value default=F("federal_agency"), output_field=CharField(), ), - "converted_organization_name" : Case( + "converted_organization_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__organization_name")), # Otherwise, return the natively assigned value default=F("organization_name"), output_field=CharField(), ), - "converted_city" : Case( + "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( + "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_so_email" : Case( + "converted_so_email": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__email")), # Otherwise, return the natively assigned senior official default=F("senior_official__email"), output_field=CharField(), ), - "converted_senior_official_last_name" : Case( + "converted_senior_official_last_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__last_name")), # Otherwise, return the natively assigned senior official default=F("senior_official__last_name"), output_field=CharField(), ), - "converted_senior_official_first_name" : Case( + "converted_senior_official_first_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__first_name")), # Otherwise, return the natively assigned senior official default=F("senior_official__first_name"), output_field=CharField(), ), - "converted_senior_official_title" : Case( + "converted_senior_official_title": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__senior_official__title")), # Otherwise, return the natively assigned senior official @@ -1360,12 +1403,15 @@ def get_computed_fields(cls, delimiter=", "): ), "converted_so_name": Case( # When portfolio is present, use that senior official instead - When(portfolio__isnull=False, then=Concat( - Coalesce(F("portfolio__senior_official__first_name"), Value("")), - Value(" "), - Coalesce(F("portfolio__senior_official__last_name"), Value("")), - output_field=CharField(), - )), + When( + portfolio__isnull=False, + then=Concat( + Coalesce(F("portfolio__senior_official__first_name"), Value("")), + Value(" "), + Coalesce(F("portfolio__senior_official__last_name"), Value("")), + output_field=CharField(), + ), + ), # Otherwise, return the natively assigned senior official default=Concat( Coalesce(F("senior_official__first_name"), Value("")), @@ -1376,25 +1422,49 @@ def get_computed_fields(cls, delimiter=", "): output_field=CharField(), ), } - + @classmethod def get_sliced_requests(cls, filter_condition): """Get filtered requests counts sliced by org type and election office.""" requests = DomainRequest.objects.all().filter(**filter_condition).distinct() requests_count = requests.count() - federal = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.FEDERAL).distinct().count() - interstate = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.INTERSTATE).distinct().count() + federal = ( + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.FEDERAL) + .distinct() + .count() + ) + interstate = ( + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.INTERSTATE) + .distinct() + .count() + ) state_or_territory = ( - cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY) + .distinct() + .count() + ) + tribal = ( + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.TRIBAL) + .distinct() + .count() + ) + county = ( + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.COUNTY) + .distinct() + .count() + ) + city = ( + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.CITY).distinct().count() ) - tribal = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.TRIBAL).distinct().count() - county = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.COUNTY).distinct().count() - city = cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.CITY).distinct().count() special_district = ( - cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SPECIAL_DISTRICT) + .distinct() + .count() ) school_district = ( - cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.SCHOOL_DISTRICT) + .distinct() + .count() ) election_board = requests.filter(is_election_board=True).distinct().count() @@ -1541,7 +1611,8 @@ def get_related_table_fields(cls): Get a list of fields from related tables. """ return ["requested_domain__name"] - + + class DomainRequestDataFull(DomainRequestExport): """ Shows all but STARTED requests @@ -1625,24 +1696,28 @@ def get_computed_fields(cls, delimiter=", "): computed_fields = super().get_computed_fields() # Add additional computed fields - computed_fields.update({ - "creator_approved_domains_count": cls.get_creator_approved_domains_count_query(), - "creator_active_requests_count": cls.get_creator_active_requests_count_query(), - "all_current_websites": StringAgg("current_websites__website", delimiter=delimiter, distinct=True), - "all_alternative_domains": StringAgg("alternative_domains__website", delimiter=delimiter, distinct=True), - # Coerce the other contacts object to "{first_name} {last_name} {email}" - "all_other_contacts": StringAgg( - Concat( - "other_contacts__first_name", - Value(" "), - "other_contacts__last_name", - Value(" "), - "other_contacts__email", + computed_fields.update( + { + "creator_approved_domains_count": cls.get_creator_approved_domains_count_query(), + "creator_active_requests_count": cls.get_creator_active_requests_count_query(), + "all_current_websites": StringAgg("current_websites__website", delimiter=delimiter, distinct=True), + "all_alternative_domains": StringAgg( + "alternative_domains__website", delimiter=delimiter, distinct=True ), - delimiter=delimiter, - distinct=True, - ), - }) + # Coerce the other contacts object to "{first_name} {last_name} {email}" + "all_other_contacts": StringAgg( + Concat( + "other_contacts__first_name", + Value(" "), + "other_contacts__last_name", + Value(" "), + "other_contacts__email", + ), + delimiter=delimiter, + distinct=True, + ), + } + ) return computed_fields diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py index d34d66daa..d9c4d192c 100644 --- a/src/registrar/views/report_views.py +++ b/src/registrar/views/report_views.py @@ -13,13 +13,9 @@ import logging - -# ---Logger -import logging -from venv import logger -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper logger = logging.getLogger(__name__) + class AnalyticsView(View): def get(self, request): thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30) @@ -165,10 +161,7 @@ def get(self, request, *args, **kwargs): class ExportDataTypeUser(View): """Returns a domain report for a given user on the request""" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"ExportDataTypeUser") - def get(self, request, *args, **kwargs): - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"ExportDataTypeUser -- get") # match the CSV example with all the fields response = HttpResponse(content_type="text/csv") response["Content-Disposition"] = 'attachment; filename="your-domains.csv"' From e75c027738cc7c21a47404ea5931ef8a515baa2a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 25 Nov 2024 15:16:04 -0700 Subject: [PATCH 13/29] Fixed annotations and sort --- src/registrar/tests/test_reports.py | 42 ++++++------- src/registrar/utility/csv_export.py | 93 ++++++++++++++++++++++------- 2 files changed, 94 insertions(+), 41 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 069042ed9..6742befb0 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -63,8 +63,8 @@ def test_generate_federal_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), call("adomain10.gov,Federal,8,,,,(blank)\r\n"), call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), ] @@ -85,8 +85,8 @@ def test_generate_full_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,183,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,183,,,,(blank)\r\n"), + call("cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), call("adomain10.gov,Federal,8,,,,(blank)\r\n"), call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), call("zdomain12.gov,Interstate,,,,,(blank)\r\n"), @@ -245,17 +245,17 @@ def test_domain_data_type(self): expected_content = ( "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO," "SO email,Security contact email,Domain managers,Invited domain managers\n" - "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,188,,,, ,,(blank),meoward@rocks.com,\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,188,,,, ,,(blank)," + "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank),meoward@rocks.com,\n" + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' - "adomain10.gov,Ready,2024-04-03,(blank),Federal,189,,,, ,,(blank),,squeaker@rocks.com\n" - "bdomain4.gov,Unknown,(blank),(blank),Federal,189,,,, ,,(blank),,\n" - "bdomain5.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" - "bdomain6.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" - "ddomain3.gov,On hold,(blank),2023-11-15,Federal,189,,,, ,,security@mail.gov,,\n" - "sdomain8.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" - "xdomain7.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" - "zdomain9.gov,Deleted,(blank),(blank),Federal,189,,,, ,,(blank),,\n" + "adomain10.gov,Ready,2024-04-03,(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,squeaker@rocks.com\n" + "bdomain4.gov,Unknown,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" + "bdomain5.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" + "bdomain6.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" + "ddomain3.gov,On hold,(blank),2023-11-15,Federal,ArmedForcesRetirementHome,,,, ,,security@mail.gov,,\n" + "sdomain8.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" + "xdomain7.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" + "zdomain9.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,squeaker@rocks.com\n" "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,\n" ) @@ -302,7 +302,7 @@ def test_domain_data_type_user(self): "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name," "City,State,SO,SO email," "Security contact email,Domain managers,Invited domain managers\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,190,,,, ,,(blank)," + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank)," '"info@example.com, meoward@rocks.com",squeaker@rocks.com\n' @@ -481,10 +481,10 @@ def test_domain_data_full(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" - "cdomain11.gov,Federal - Executive,186,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,186,,,,(blank)\n" - "adomain10.gov,Federal,187,,,,(blank)\n" - "ddomain3.gov,Federal,187,,,,security@mail.gov\n" + "cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" + "defaultsecurity.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" + "adomain10.gov,Federal,WorldWarICentennialCommission,,,,(blank)\n" + "ddomain3.gov,Federal,WorldWarICentennialCommission,,,,security@mail.gov\n" "zdomain12.gov,Interstate,,,,,(blank)\n" ) @@ -524,8 +524,8 @@ def test_domain_data_federal(self): "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" "cdomain11.gov,Federal - Executive,184,,,,(blank)\n" "defaultsecurity.gov,Federal - Executive,184,,,,(blank)\n" - "adomain10.gov,Federal,185,,,,(blank)\n" - "ddomain3.gov,Federal,185,,,,security@mail.gov\n" + "adomain10.gov,Federal,WorldWarICentennialCommission,,,,(blank)\n" + "ddomain3.gov,Federal,WorldWarICentennialCommission,,,,security@mail.gov\n" ) # Normalize line endings and remove commas, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index fff55baed..21088424e 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -302,16 +302,31 @@ def get_computed_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ - # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # NOTE: These computed fields imitate @Property functions in the Domain model and Portfolio model where needed. # This is for performance purposes. Since we are working with dictionary values and not # model objects as we export data, trying to reinstate model objects in order to grab @property # values negatively impacts performance. Therefore, we will follow best practice and use annotations return { + "converted_generic_org_type": Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__organization_type")), + # Otherwise, return the natively assigned value + default=F("organization_type"), + output_field=CharField(), + ), "converted_federal_agency": Case( # When portfolio is present, use its value instead - When(portfolio__isnull=False, then=F("portfolio__federal_agency")), + When(portfolio__isnull=False, then=F("portfolio__federal_agency__agency")), # Otherwise, return the natively assigned value - default=F("federal_agency"), + default=F("federal_agency__agency"), + output_field=CharField(), + ), + "converted_federal_type": Case( + # When portfolio is present, use its value instead + # NOTE: this is an @Property funciton in portfolio. + When(Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), then=F("portfolio__federal_agency__federal_type")), + # Otherwise, return the natively assigned value + default=F("federal_type"), output_field=CharField(), ), "converted_organization_name": Case( @@ -475,9 +490,9 @@ def parse_row(cls, columns, model): first_ready_on = "(blank)" # organization_type has organization_type AND is_election - domain_org_type = model.get("converted_generic_org_type") or model.get("organization_type") + domain_org_type = model.get("converted_generic_org_type") human_readable_domain_org_type = DomainRequest.OrgChoicesElectionOffice.get_org_label(domain_org_type) - domain_federal_type = model.get("federal_type") + domain_federal_type = model.get("converted_federal_type") human_readable_domain_federal_type = BranchChoices.get_branch_label(domain_federal_type) domain_type = human_readable_domain_org_type if domain_federal_type and domain_org_type == DomainRequest.OrgChoicesElectionOffice.FEDERAL: @@ -623,6 +638,14 @@ def get_columns(cls): "Domain managers", "Invited domain managers", ] + + + @classmethod + def get_annotations_for_sort(cls, delimiter=", "): + """ + Get a dict of annotations to make available for sorting. + """ + return cls.get_computed_fields() @classmethod def get_sort_fields(cls): @@ -631,9 +654,9 @@ def get_sort_fields(cls): """ # Coalesce is used to replace federal_type of None with ZZZZZ return [ - "organization_type", - Coalesce("federal_type", Value("ZZZZZ")), - "federal_agency", + "converted_generic_org_type", + Coalesce("converted_federal_type", Value("ZZZZZ")), + "converted_federal_agency", "domain__name", ] @@ -779,7 +802,7 @@ def exporting_dr_data_to_csv(cls, response, request=None): cls.safe_get(getattr(request, "region_field", None)), request.status, cls.safe_get(getattr(request, "election_office", None)), - request.federal_type, + request.converted_federal_type, cls.safe_get(getattr(request, "domain_type", None)), cls.safe_get(getattr(request, "additional_details", None)), cls.safe_get(getattr(request, "creator_approved_domains_count", None)), @@ -829,6 +852,13 @@ def get_columns(cls): "State", "Security contact email", ] + + @classmethod + def get_annotations_for_sort(cls, delimiter=", "): + """ + Get a dict of annotations to make available for sorting. + """ + return cls.get_computed_fields() @classmethod def get_sort_fields(cls): @@ -837,9 +867,9 @@ def get_sort_fields(cls): """ # Coalesce is used to replace federal_type of None with ZZZZZ return [ - "organization_type", - Coalesce("federal_type", Value("ZZZZZ")), - "federal_agency", + "converted_generic_org_type", + Coalesce("converted_federal_type", Value("ZZZZZ")), + "converted_federal_agency", "domain__name", ] @@ -910,6 +940,14 @@ def get_columns(cls): "Security contact email", ] + + @classmethod + def get_annotations_for_sort(cls, delimiter=", "): + """ + Get a dict of annotations to make available for sorting. + """ + return cls.get_computed_fields() + @classmethod def get_sort_fields(cls): """ @@ -917,9 +955,9 @@ def get_sort_fields(cls): """ # Coalesce is used to replace federal_type of None with ZZZZZ return [ - "organization_type", - Coalesce("federal_type", Value("ZZZZZ")), - "federal_agency", + "converted_generic_org_type", + Coalesce("converted_federal_type", Value("ZZZZZ")), + "converted_federal_agency", "domain__name", ] @@ -1340,16 +1378,31 @@ def get_computed_fields(cls, delimiter=", "): """ Get a dict of computed fields. """ - # NOTE: These computed fields imitate @Property functions in the Domain model where needed. + # NOTE: These computed fields imitate @Property functions in the Domain model and Portfolio model where needed. # This is for performance purposes. Since we are working with dictionary values and not # model objects as we export data, trying to reinstate model objects in order to grab @property # values negatively impacts performance. Therefore, we will follow best practice and use annotations return { + "converted_generic_org_type": Case( + # When portfolio is present, use its value instead + When(portfolio__isnull=False, then=F("portfolio__organization_type")), + # Otherwise, return the natively assigned value + default=F("organization_type"), + output_field=CharField(), + ), "converted_federal_agency": Case( # When portfolio is present, use its value instead - When(portfolio__isnull=False, then=F("portfolio__federal_agency")), + When(portfolio__isnull=False, then=F("portfolio__federal_agency__agency")), + # Otherwise, return the natively assigned value + default=F("federal_agency__agency"), + output_field=CharField(), + ), + "converted_federal_type": Case( + # When portfolio is present, use its value instead + # NOTE: this is an @Property funciton in portfolio. + When(Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), then=F("portfolio__federal_agency__federal_type")), # Otherwise, return the natively assigned value - default=F("federal_agency"), + default=F("federal_type"), output_field=CharField(), ), "converted_organization_name": Case( @@ -1488,11 +1541,11 @@ def parse_row(cls, columns, model): """ # Handle the federal_type field. Defaults to the wrong format. - federal_type = model.get("federal_type") + federal_type = model.get("converted_federal_type") human_readable_federal_type = BranchChoices.get_branch_label(federal_type) if federal_type else None # Handle the org_type field - org_type = model.get("converted_generic_org_type") or model.get("organization_type") + org_type = model.get("converted_generic_org_type") human_readable_org_type = DomainRequest.OrganizationChoices.get_org_label(org_type) if org_type else None # Handle the status field. Defaults to the wrong format. From 0446aaba60adec8efe45f510ebcafc6357090d80 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 25 Nov 2024 15:26:22 -0700 Subject: [PATCH 14/29] unit test fixes --- src/registrar/tests/test_reports.py | 41 +++++++++++++++-------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 6742befb0..ff72359d6 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -63,10 +63,10 @@ def test_generate_federal_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), - call("adomain10.gov,Federal,8,,,,(blank)\r\n"), - call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), + call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), + call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), + call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), ] # We don't actually want to write anything for a test case, # we just want to verify what is being written. @@ -85,10 +85,10 @@ def test_generate_full_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), - call("cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\r\n"), - call("adomain10.gov,Federal,8,,,,(blank)\r\n"), - call("ddomain3.gov,Federal,8,,,,(blank)\r\n"), + call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), + call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), + call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), + call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), call("zdomain12.gov,Interstate,,,,,(blank)\r\n"), ] # We don't actually want to write anything for a test case, @@ -483,8 +483,8 @@ def test_domain_data_full(self): "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" "cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" "defaultsecurity.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" - "adomain10.gov,Federal,WorldWarICentennialCommission,,,,(blank)\n" - "ddomain3.gov,Federal,WorldWarICentennialCommission,,,,security@mail.gov\n" + "adomain10.gov,Federal,ArmedForcesRetirementHome,,,,(blank)\n" + "ddomain3.gov,Federal,ArmedForcesRetirementHome,,,,security@mail.gov\n" "zdomain12.gov,Interstate,,,,,(blank)\n" ) @@ -522,10 +522,10 @@ def test_domain_data_federal(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" - "cdomain11.gov,Federal - Executive,184,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,184,,,,(blank)\n" - "adomain10.gov,Federal,WorldWarICentennialCommission,,,,(blank)\n" - "ddomain3.gov,Federal,WorldWarICentennialCommission,,,,security@mail.gov\n" + "cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" + "defaultsecurity.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" + "adomain10.gov,Federal,ArmedForcesRetirementHome,,,,(blank)\n" + "ddomain3.gov,Federal,ArmedForcesRetirementHome,,,,security@mail.gov\n" ) # Normalize line endings and remove commas, @@ -539,6 +539,7 @@ def test_domain_data_federal(self): def test_domain_growth(self): """Shows ready and deleted domains within a date range, sorted""" # Remove "Created at" and "First ready" because we can't guess this immutable, dynamically generated test data + self.maxDiff=None columns = [ "Domain name", "Domain type", @@ -577,13 +578,13 @@ def test_domain_growth(self): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date, Deleted\n" - "cdomain1.gov,Federal-Executive,194,Ready,(blank)\n" - "adomain10.gov,Federal,195,Ready,(blank)\n" - "cdomain11.gov,Federal-Executive,194,Ready,(blank)\n" + "cdomain1.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank)\n" + "adomain10.gov,Federal,ArmedForcesRetirementHome,Ready,(blank)\n" + "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank)\n" "zdomain12.gov,Interstate,Ready,(blank)\n" - "zdomain9.gov,Federal,195,Deleted,(blank),2024-04-01\n" - "sdomain8.gov,Federal,195,Deleted,(blank),2024-04-02\n" - "xdomain7.gov,Federal,195,Deleted,(blank),2024-04-02\n" + "zdomain9.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-01\n" + "sdomain8.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-02\n" + "xdomain7.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-02\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace From 31563152da982ee05c84421500ef7042f4ea2788 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 25 Nov 2024 15:32:53 -0700 Subject: [PATCH 15/29] cleanup --- src/registrar/admin.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 56b5fa4fe..0bdcb4da8 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2809,17 +2809,6 @@ def queryset(self, request, queryset): ) return queryset - def get_queryset(self, request): - """Custom get_queryset to filter by portfolio if portfolio is in the - request params.""" - qs = super().get_queryset(request) - # Check if a 'portfolio' parameter is passed in the request - portfolio_id = request.GET.get("portfolio") - if portfolio_id: - # Further filter the queryset by the portfolio - qs = qs.filter(domain_info__portfolio=portfolio_id) - return qs - # Filters list_filter = [GenericOrgFilter, FederalTypeFilter, ElectionOfficeFilter, "state"] @@ -3227,6 +3216,17 @@ def has_change_permission(self, request, obj=None): ): return True return super().has_change_permission(request, obj) + + def get_queryset(self, request): + """Custom get_queryset to filter by portfolio if portfolio is in the + request params.""" + qs = super().get_queryset(request) + # Check if a 'portfolio' parameter is passed in the request + portfolio_id = request.GET.get("portfolio") + if portfolio_id: + # Further filter the queryset by the portfolio + qs = qs.filter(domain_info__portfolio=portfolio_id) + return qs class DraftDomainResource(resources.ModelResource): From c0a0ac3ed214bf5e7f24849fad41f74e0799418d Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 25 Nov 2024 15:35:00 -0700 Subject: [PATCH 16/29] linted --- src/registrar/admin.py | 2 +- src/registrar/tests/test_reports.py | 20 +++++++++++++------- src/registrar/utility/csv_export.py | 16 ++++++++++------ 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 0bdcb4da8..168b266cf 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3216,7 +3216,7 @@ def has_change_permission(self, request, obj=None): ): return True return super().has_change_permission(request, obj) - + def get_queryset(self, request): """Custom get_queryset to filter by portfolio if portfolio is in the request params.""" diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index ff72359d6..7ab4bf020 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -245,18 +245,23 @@ def test_domain_data_type(self): expected_content = ( "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO," "SO email,Security contact email,Domain managers,Invited domain managers\n" - "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank),meoward@rocks.com,\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank)," + "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,WorldWarICentennialCommission" + ",,,, ,,(blank),meoward@rocks.com,\n" + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission" + ",,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' - "adomain10.gov,Ready,2024-04-03,(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,squeaker@rocks.com\n" + "adomain10.gov,Ready,2024-04-03,(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),," + "squeaker@rocks.com\n" "bdomain4.gov,Unknown,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" "bdomain5.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" "bdomain6.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "ddomain3.gov,On hold,(blank),2023-11-15,Federal,ArmedForcesRetirementHome,,,, ,,security@mail.gov,,\n" + "ddomain3.gov,On hold,(blank),2023-11-15,Federal,ArmedForcesRetirementHome,,,, ,," + "security@mail.gov,,\n" "sdomain8.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" "xdomain7.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" "zdomain9.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,squeaker@rocks.com\n" + "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank),meoward@rocks.com," + "squeaker@rocks.com\n" "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,\n" ) @@ -302,7 +307,8 @@ def test_domain_data_type_user(self): "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name," "City,State,SO,SO email," "Security contact email,Domain managers,Invited domain managers\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission,,,, ,,(blank)," + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive," + "WorldWarICentennialCommission,,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank)," '"info@example.com, meoward@rocks.com",squeaker@rocks.com\n' @@ -539,7 +545,7 @@ def test_domain_data_federal(self): def test_domain_growth(self): """Shows ready and deleted domains within a date range, sorted""" # Remove "Created at" and "First ready" because we can't guess this immutable, dynamically generated test data - self.maxDiff=None + self.maxDiff = None columns = [ "Domain name", "Domain type", diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 21088424e..a8bb9471d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -324,7 +324,10 @@ def get_computed_fields(cls, delimiter=", "): "converted_federal_type": Case( # When portfolio is present, use its value instead # NOTE: this is an @Property funciton in portfolio. - When(Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), then=F("portfolio__federal_agency__federal_type")), + When( + Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), + then=F("portfolio__federal_agency__federal_type"), + ), # Otherwise, return the natively assigned value default=F("federal_type"), output_field=CharField(), @@ -638,7 +641,6 @@ def get_columns(cls): "Domain managers", "Invited domain managers", ] - @classmethod def get_annotations_for_sort(cls, delimiter=", "): @@ -852,7 +854,7 @@ def get_columns(cls): "State", "Security contact email", ] - + @classmethod def get_annotations_for_sort(cls, delimiter=", "): """ @@ -940,14 +942,13 @@ def get_columns(cls): "Security contact email", ] - @classmethod def get_annotations_for_sort(cls, delimiter=", "): """ Get a dict of annotations to make available for sorting. """ return cls.get_computed_fields() - + @classmethod def get_sort_fields(cls): """ @@ -1400,7 +1401,10 @@ def get_computed_fields(cls, delimiter=", "): "converted_federal_type": Case( # When portfolio is present, use its value instead # NOTE: this is an @Property funciton in portfolio. - When(Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), then=F("portfolio__federal_agency__federal_type")), + When( + Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), + then=F("portfolio__federal_agency__federal_type"), + ), # Otherwise, return the natively assigned value default=F("federal_type"), output_field=CharField(), From 36c19d65bf4edef8c74707123f4867f5eeae4e8f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 26 Nov 2024 13:31:01 -0700 Subject: [PATCH 17/29] fixes for unit test (and admin change form) --- src/registrar/admin.py | 2 +- src/registrar/tests/test_admin_domain.py | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 168b266cf..8959c8f8d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1579,7 +1579,7 @@ def converted_generic_org_type(self, obj): }, ), (".gov domain", {"fields": ["domain"]}), - ("Contacts", {"fields": ["generic_org_type", "other_contacts", "no_other_contacts_rationale"]}), + ("Contacts", {"fields": ["senior_official", "other_contacts", "no_other_contacts_rationale"]}), ("Background info", {"fields": ["anything_else"]}), ( "Type of organization", diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index f02b59a91..3e716c247 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -32,9 +32,6 @@ import boto3_mocking # type: ignore import logging -logger = logging.getLogger(__name__) - - class TestDomainAdminAsStaff(MockEppLib): """Test DomainAdmin class as staff user. @@ -494,7 +491,7 @@ def test_has_model_description(self): self.assertContains(response, "This table contains all approved domains in the .gov registrar.") self.assertContains(response, "Show more") - @less_console_noise_decorator + # @less_console_noise_decorator def test_contact_fields_on_domain_change_form_have_detail_table(self): """Tests if the contact fields in the inlined Domain information have the detail table which displays title, email, and phone""" @@ -726,9 +723,9 @@ def test_short_org_name_in_domains_list(self): domain_request.approve() response = self.client.get("/admin/registrar/domain/") - # There are 4 template references to Federal (4) plus four references in the table - # for our actual domain_request - self.assertContains(response, "Federal", count=56) + # The total count should reflect the fact that we are pulling from portfolio + # data when portfolios are present + self.assertContains(response, "Federal", count=98) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist From e7c22ce51f4517b2f14df37af5b50db02a2126b7 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 26 Nov 2024 14:40:43 -0700 Subject: [PATCH 18/29] linted --- .../models/user_portfolio_permission.py | 9 +++++-- .../models/utility/portfolio_helper.py | 2 ++ src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/utility/csv_export.py | 24 ++----------------- src/registrar/utility/enums.py | 1 + src/registrar/views/portfolio_members_json.py | 2 -- 6 files changed, 13 insertions(+), 27 deletions(-) diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py index 51f3fa3fe..319f15d67 100644 --- a/src/registrar/models/user_portfolio_permission.py +++ b/src/registrar/models/user_portfolio_permission.py @@ -2,7 +2,12 @@ from django.forms import ValidationError from registrar.models.user_domain_role import UserDomainRole from registrar.utility.waffle import flag_is_active_for_user -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices, DomainRequestPermissionDisplay, MemberPermissionDisplay +from registrar.models.utility.portfolio_helper import ( + UserPortfolioPermissionChoices, + UserPortfolioRoleChoices, + DomainRequestPermissionDisplay, + MemberPermissionDisplay, +) from .utility.time_stamped_model import TimeStampedModel from django.contrib.postgres.fields import ArrayField @@ -115,7 +120,7 @@ def get_domain_request_permission_display(cls, roles, additional_permissions): UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, UserPortfolioPermissionChoices.EDIT_REQUESTS, ] - + if all(perm in all_permissions for perm in all_domain_perms): return DomainRequestPermissionDisplay.VIEWER_REQUESTER elif UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS in all_permissions: diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py index 9b661b316..f1a6cec7a 100644 --- a/src/registrar/models/utility/portfolio_helper.py +++ b/src/registrar/models/utility/portfolio_helper.py @@ -51,6 +51,7 @@ class DomainRequestPermissionDisplay(StrEnum): - VIEWER: "Viewer" - NONE: "None" """ + VIEWER_REQUESTER = "Viewer Requester" VIEWER = "Viewer" NONE = "None" @@ -64,6 +65,7 @@ class MemberPermissionDisplay(StrEnum): - VIEWER: "Viewer" - NONE: "None" """ + MANAGER = "Manager" VIEWER = "Viewer" NONE = "None" diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 3e716c247..e1d6ffcb1 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -30,7 +30,7 @@ from unittest.mock import ANY, call, patch import boto3_mocking # type: ignore -import logging + class TestDomainAdminAsStaff(MockEppLib): """Test DomainAdmin class as staff user. diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1bfd5d02e..b67778655 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -198,8 +198,8 @@ def annotate_and_retrieve_fields( # We can infer that if we're passing in annotations, # we want to grab the result of said annotation. - if computed_fields : - related_table_fields.extend(computed_fields .keys()) + if computed_fields: + related_table_fields.extend(computed_fields.keys()) # Get prexisting fields on the model model_fields = set() @@ -213,26 +213,6 @@ def annotate_and_retrieve_fields( return cls.update_queryset(queryset, **kwargs) - @classmethod - def export_data_to_csv(cls, csv_file, **kwargs): - """ - All domain metadata: - Exports domains of all statuses plus domain managers. - """ - - writer = csv.writer(csv_file) - columns = cls.get_columns() - models_dict = cls.get_model_annotation_dict(**kwargs) - - # Write to csv file before the write_csv - cls.write_csv_before(writer, **kwargs) - - # Write the csv file - rows = cls.write_csv(writer, columns, models_dict) - - # Return rows that for easier parsing and testing - return rows - @classmethod def get_annotated_queryset(cls, **kwargs): """Returns an annotated queryset based off of all query conditions.""" diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 232c4056f..47e6da47f 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -49,6 +49,7 @@ class DefaultUserValues(StrEnum): - SYSTEM: "System" <= Default username - UNRETRIEVED: "Unretrieved" <= Default email state """ + HELP_EMAIL = "help@get.gov" SYSTEM = "System" UNRETRIEVED = "Unretrieved" diff --git a/src/registrar/views/portfolio_members_json.py b/src/registrar/views/portfolio_members_json.py index 232ca2e6c..b5c608eab 100644 --- a/src/registrar/views/portfolio_members_json.py +++ b/src/registrar/views/portfolio_members_json.py @@ -1,7 +1,6 @@ from django.http import JsonResponse from django.core.paginator import Paginator from django.db.models import Value, F, CharField, TextField, Q, Case, When, OuterRef, Subquery -from django.db.models.expressions import Func from django.db.models.functions import Cast, Coalesce, Concat from django.contrib.postgres.aggregates import ArrayAgg from django.urls import reverse @@ -214,4 +213,3 @@ def serialize_members(self, request, portfolio, item, user): "svg_icon": ("visibility" if view_only else "settings"), } return member_json - From 6ca53560eadc63105eda00edd20305f370049240 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Nov 2024 15:17:09 -0700 Subject: [PATCH 19/29] Re-added missing function (merge error) --- src/registrar/utility/csv_export.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index af49ac663..33ba28b61 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -243,6 +243,25 @@ def get_annotated_queryset(cls, **kwargs): @classmethod def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) + + @classmethod + def export_data_to_csv(cls, csv_file, **kwargs): + """ + All domain metadata: + Exports domains of all statuses plus domain managers. + """ + writer = csv.writer(csv_file) + columns = cls.get_columns() + models_dict = cls.get_model_annotation_dict(**kwargs) + + # Write to csv file before the write_csv + cls.write_csv_before(writer, **kwargs) + + # Write the csv file + rows = cls.write_csv(writer, columns, models_dict) + + # Return rows that for easier parsing and testing + return rows @classmethod def write_csv( From a6308edffe91dd63ca64a17b86b7617060276127 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 2 Dec 2024 13:00:47 -0700 Subject: [PATCH 20/29] A few updates based on feedback --- src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/tests/test_reports.py | 6 +----- src/registrar/utility/csv_export.py | 10 +++++----- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index e1d6ffcb1..fb1511e1d 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -491,7 +491,7 @@ def test_has_model_description(self): self.assertContains(response, "This table contains all approved domains in the .gov registrar.") self.assertContains(response, "Show more") - # @less_console_noise_decorator + @less_console_noise_decorator def test_contact_fields_on_domain_change_form_have_detail_table(self): """Tests if the contact fields in the inlined Domain information have the detail table which displays title, email, and phone""" diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 01fe2848f..846fa5915 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -326,7 +326,6 @@ def test_domain_data_type_user(self): # 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) @@ -509,7 +508,7 @@ def test_domain_data_full(self): self.maxDiff = None self.assertEqual(csv_content, expected_content) - # @less_console_noise_decorator + @less_console_noise_decorator def test_domain_data_federal(self): """Shows security contacts, filtered by state and org type""" # Add security email information @@ -606,7 +605,6 @@ def test_domain_growth(self): csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() ) expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.assertEqual(csv_content, expected_content) @less_console_noise_decorator @@ -689,7 +687,6 @@ def test_domain_unmanaged(self): # 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.assertEqual(csv_content, expected_content) @less_console_noise_decorator @@ -727,7 +724,6 @@ def test_domain_request_growth(self): # 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.assertEqual(csv_content, expected_content) @less_console_noise_decorator diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 33ba28b61..e493c8715 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -138,7 +138,7 @@ def get_filter_conditions(cls, **kwargs): return Q() @classmethod - def get_computed_fields(cls, **kwargs): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -526,7 +526,7 @@ def model(cls): return DomainInformation @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -612,7 +612,7 @@ def get_computed_fields(cls, delimiter=", "): "converted_so_name": Case( # When portfolio is present, use that senior official instead When( - portfolio__isnull=False, + Q(portfolio__isnull=False) & Q(portfolio__senior_official__isnull=False), then=Concat( Coalesce(F("portfolio__senior_official__first_name"), Value("")), Value(" "), @@ -1615,7 +1615,7 @@ def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter ) @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1701,7 +1701,7 @@ def get_computed_fields(cls, delimiter=", "): "converted_so_name": Case( # When portfolio is present, use that senior official instead When( - portfolio__isnull=False, + Q(portfolio__isnull=False) & Q(portfolio__senior_official__isnull=False), then=Concat( Coalesce(F("portfolio__senior_official__first_name"), Value("")), Value(" "), From 9a8ac325e58ba6044f21f427bee051a06b55a483 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 3 Dec 2024 15:03:05 -0700 Subject: [PATCH 21/29] Remove converted values from exports + fixes --- src/registrar/admin.py | 297 +++++++-------------- src/registrar/models/domain_information.py | 37 ++- src/registrar/utility/csv_export.py | 13 +- 3 files changed, 133 insertions(+), 214 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b7d6517a8..8f345814d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -3,7 +3,14 @@ import copy from typing import Optional from django import forms -from django.db.models import Value, CharField, Q +from django.db.models import ( + Case, + CharField, + F, + Q, + Value, + When, +) from django.db.models.functions import Concat, Coalesce from django.http import HttpResponseRedirect from registrar.models.federal_agency import FederalAgency @@ -1463,55 +1470,6 @@ class DomainInformationResource(resources.ModelResource): class Meta: model = models.DomainInformation - # Override exports for these columns in DomainInformation to use converted values. These values - # come from @Property functions, which are not automatically included in the export and which we - # want to use in place of the native fields. - organization_name = fields.Field(attribute="converted_organization_name", column_name="organization_name") - generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic_org_type") - federal_type = fields.Field(attribute="converted_federal_type", column_name="federal_type") - federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal_agency") - senior_official = fields.Field(attribute="converted_senior_official", column_name="senior_official") - address_line1 = fields.Field(attribute="converted_address_line1", column_name="address_line1") - address_line2 = fields.Field(attribute="converted_address_line2", column_name="address_line2") - city = fields.Field(attribute="converted_city", column_name="city") - state_territory = fields.Field(attribute="converted_state_territory", column_name="state_territory") - zipcode = fields.Field(attribute="converted_zipcode", column_name="zipcode") - urbanization = fields.Field(attribute="converted_urbanization", column_name="urbanization") - - # Custom getters for the above columns that map to @property functions instead of fields - def dehydrate_organization_name(self, obj): - return obj.converted_organization_name - - def dehydrate_generic_org_type(self, obj): - return obj.converted_generic_org_type - - def dehydrate_federal_type(self, obj): - return obj.converted_federal_type - - def dehydrate_federal_agency(self, obj): - return obj.converted_federal_agency - - def dehydrate_senior_official(self, obj): - return obj.converted_senior_official - - def dehydrate_address_line1(self, obj): - return obj.converted_address_line1 - - def dehydrate_address_line2(self, obj): - return obj.converted_address_line2 - - def dehydrate_city(self, obj): - return obj.converted_city - - def dehydrate_state_territory(self, obj): - return obj.converted_state_territory - - def dehydrate_zipcode(self, obj): - return obj.converted_zipcode - - def dehydrate_urbanization(self, obj): - return obj.converted_urbanization - class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Customize domain information admin class.""" @@ -1527,25 +1485,26 @@ class GenericOrgFilter(admin.SimpleListFilter): def lookups(self, request, model_admin): converted_generic_orgs = set() - for domainInfo in DomainInformation.objects.all(): - converted_generic_org = domainInfo.converted_generic_org_type + # Populate the set with tuples of (value, display value) + for domain_info in DomainInformation.objects.all(): + converted_generic_org = domain_info.converted_generic_org_type # Actual value + converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value + if converted_generic_org: - converted_generic_orgs.add(converted_generic_org) + converted_generic_orgs.add( + (converted_generic_org, converted_generic_org_display) # Value, Display + ) - return sorted((org, org) for org in converted_generic_orgs) + # Sort the set by display value + return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value # Filter queryset def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domainInfo.id - for domainInfo in queryset - if domainInfo.converted_generic_org_type - and domainInfo.converted_generic_org_type == self.value() - ] - ) + Q(portfolio__organization_type=self.value()) | + Q(generic_org_type=self.value()) + ) return queryset resource_classes = [DomainInformationResource] @@ -1707,6 +1666,8 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): # 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): @@ -1716,56 +1677,6 @@ class DomainRequestResource(FsmModelResource): class Meta: model = models.DomainRequest - # Override exports for these columns in DomainInformation to use converted values. These values - # come from @Property functions, which are not automatically included in the export and which we - # want to use in place of the native fields. - organization_name = fields.Field(attribute="converted_organization_name", column_name="organization_name") - generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic_org_type") - federal_type = fields.Field(attribute="converted_federal_type", column_name="federal_type") - federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal_agency") - senior_official = fields.Field(attribute="converted_senior_official", column_name="senior_official") - address_line1 = fields.Field(attribute="converted_address_line1", column_name="address_line1") - address_line2 = fields.Field(attribute="converted_address_line2", column_name="address_line2") - city = fields.Field(attribute="converted_city", column_name="city") - state_territory = fields.Field(attribute="converted_state_territory", column_name="state_territory") - zipcode = fields.Field(attribute="converted_zipcode", column_name="zipcode") - urbanization = fields.Field(attribute="converted_urbanization", column_name="urbanization") - senior_official = fields.Field(attribute="converted_urbanization", column_name="senior official") - - # Custom getters for the above columns that map to @property functions instead of fields - def dehydrate_organization_name(self, obj): - return obj.converted_organization_name - - def dehydrate_generic_org_type(self, obj): - return obj.converted_generic_org_type - - def dehydrate_federal_type(self, obj): - return obj.converted_federal_type - - def dehydrate_federal_agency(self, obj): - return obj.converted_federal_agency - - def dehydrate_senior_official(self, obj): - return obj.converted_senior_official - - def dehydrate_address_line1(self, obj): - return obj.converted_address_line1 - - def dehydrate_address_line2(self, obj): - return obj.converted_address_line2 - - def dehydrate_city(self, obj): - return obj.converted_city - - def dehydrate_state_territory(self, obj): - return obj.converted_state_territory - - def dehydrate_zipcode(self, obj): - return obj.converted_zipcode - - def dehydrate_urbanization(self, obj): - return obj.converted_urbanization - class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): """Custom domain requests admin class.""" @@ -1797,25 +1708,26 @@ class GenericOrgFilter(admin.SimpleListFilter): def lookups(self, request, model_admin): converted_generic_orgs = set() - for domain_request in DomainRequest.objects.all(): - converted_generic_org = domain_request.converted_generic_org_type + # Populate the set with tuples of (value, display value) + for domain_info in DomainInformation.objects.all(): + converted_generic_org = domain_info.converted_generic_org_type # Actual value + converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value + if converted_generic_org: - converted_generic_orgs.add(converted_generic_org) + converted_generic_orgs.add( + (converted_generic_org, converted_generic_org_display) # Value, Display + ) - return sorted((org, org) for org in converted_generic_orgs) + # Sort the set by display value + return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value # Filter queryset def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domain_request.id - for domain_request in queryset - if domain_request.converted_generic_org_type - and domain_request.converted_generic_org_type == self.value() - ] - ) + Q(portfolio__organization_type=self.value()) | + Q(generic_org_type=self.value()) + ) return queryset class FederalTypeFilter(admin.SimpleListFilter): @@ -1838,16 +1750,11 @@ def lookups(self, request, model_admin): # Filter queryset def queryset(self, request, queryset): - if self.value(): # Check if federal Type is selected in the filter + if self.value(): # Check if a federal type is selected in the filter return queryset.filter( - # Filter based on the federal type returned by converted_federal_type - id__in=[ - domain_request.id - for domain_request in queryset - if domain_request.converted_federal_type - and domain_request.converted_federal_type == self.value() - ] - ) + Q(portfolio__federal_type=self.value()) | + Q(federal_type=self.value()) + ) return queryset class InvestigatorFilter(admin.SimpleListFilter): @@ -2808,45 +2715,6 @@ class DomainResource(FsmModelResource): class Meta: model = models.Domain - # Override the default export so that it matches what is displayed in the admin table for Domains - fields = ( - "name", - "converted_generic_org_type", - "federal_type", - "converted_federal_type", - "converted_federal_agency", - "converted_organization_name", - "custom_election_board", - "converted_city", - "converted_state_territory", - "state", - "expiration_date", - "created_at", - "first_ready", - "deleted", - ) - - # Custom getters to retrieve the values from @Proprerty methods in DomainInfo - converted_generic_org_type = fields.Field(attribute="converted_generic_org_type", column_name="generic org type") - converted_federal_agency = fields.Field(attribute="converted_federal_agency", column_name="federal agency") - converted_organization_name = fields.Field(attribute="converted_organization_name", column_name="organization name") - converted_city = fields.Field(attribute="converted_city", column_name="city") - converted_state_territory = fields.Field(attribute="converted_state_territory", column_name="state territory") - - def dehydrate_converted_generic_org_type(self, obj): - return obj.domain_info.converted_generic_org_type - - def dehydrate_converted_federal_agency(self, obj): - return obj.domain_info.converted_federal_agency - - def dehydrate_converted_organization_name(self, obj): - return obj.domain_info.converted_organization_name - - def dehydrate_converted_city(self, obj): - return obj.domain_info.converted_city - - def dehydrate_converted_state_territory(self, obj): - return obj.domain_info.converted_state_territory class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @@ -2884,26 +2752,68 @@ class GenericOrgFilter(admin.SimpleListFilter): def lookups(self, request, model_admin): converted_generic_orgs = set() - for domainInfo in DomainInformation.objects.all(): - converted_generic_org = domainInfo.converted_generic_org_type + # Populate the set with tuples of (value, display value) + for domain_info in DomainInformation.objects.all(): + converted_generic_org = domain_info.converted_generic_org_type # Actual value + converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value + if converted_generic_org: - converted_generic_orgs.add(converted_generic_org) + converted_generic_orgs.add( + (converted_generic_org, converted_generic_org_display) # Value, Display + ) + + # Sort the set by display value + return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value - return sorted((org, org) for org in converted_generic_orgs) # Filter queryset def queryset(self, request, queryset): + + annotated_queryset = queryset.annotate( + converted_generic_org_type=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_type")), + # Otherwise, return the natively assigned value + default=F("domain_info__generic_org_type"), + ), + converted_federal_agency=Case( + # When portfolio is present, use its value instead + When( + Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), + then=F("domain_info__portfolio__federal_agency__agency") + ), + # Otherwise, return the natively assigned value + default=F("domain_info__federal_agency__agency"), + ), + converted_organization_name=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_name")), + # Otherwise, return the natively assigned value + default=F("domain_info__organization_name"), + ), + converted_city=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__city")), + # Otherwise, return the natively assigned value + default=F("domain_info__city"), + ), + converted_state_territory=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__state_territory")), + # Otherwise, return the natively assigned value + default=F("domain_info__state_territory"), + ), + ) + if self.value(): # Check if a generic org is selected in the filter - return queryset.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domain.id - for domain in queryset - if domain.domain_info.converted_generic_org_type - and domain.domain_info.converted_generic_org_type == self.value() - ] - ) - return queryset + return annotated_queryset.filter( + Q(domain_info__portfolio__organization_type=self.value()) | + Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) + ) + + return annotated_queryset + + class FederalTypeFilter(admin.SimpleListFilter): """Custom Federal Type filter that accomodates portfolio feature. @@ -2926,16 +2836,11 @@ def lookups(self, request, model_admin): # Filter queryset def queryset(self, request, queryset): - if self.value(): # Check if a generic org is selected in the filter + if self.value(): # Check if a federal type is selected in the filter return queryset.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domain.id - for domain in queryset - if domain.domain_info.converted_federal_type - and domain.domain_info.converted_federal_type == self.value() - ] - ) + Q(portfolio__federal_type=self.value()) | + Q(federal_type=self.value()) + ) return queryset # Filters @@ -2976,7 +2881,7 @@ def queryset(self, request, queryset): # Use converted value in the table @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): - return obj.domain_info.converted_generic_org_type + return obj.domain_info.converted_generic_org_type_display converted_generic_org_type.admin_order_field = "domain_info__converted_generic_org_type" # type: ignore diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index db5416cc2..60c4d8c75 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -426,6 +426,7 @@ def get_state_display_of_domain(self): else: return None + # ----- Portfolio Properties ----- @property @@ -433,7 +434,7 @@ def converted_organization_name(self): if self.portfolio: return self.portfolio.organization_name return self.organization_name - + @property def converted_generic_org_type(self): if self.portfolio: @@ -450,25 +451,25 @@ def converted_federal_agency(self): def converted_federal_type(self): if self.portfolio: return self.portfolio.federal_type - return self.federal_type + return self.get_federal_type_display() @property def converted_senior_official(self): if self.portfolio: - return self.portfolio.senior_official - return self.senior_official + return self.portfolio.display_senior_official + return self.display_senior_official @property def converted_address_line1(self): if self.portfolio: - return self.portfolio.address_line1 - return self.address_line1 + return self.portfolio.display_address_line1 + return self.display_address_line1 @property def converted_address_line2(self): if self.portfolio: - return self.portfolio.address_line2 - return self.address_line2 + return self.portfolio.display_address_line2 + return self.display_address_line2 @property def converted_city(self): @@ -479,17 +480,25 @@ def converted_city(self): @property def converted_state_territory(self): if self.portfolio: - return self.portfolio.state_territory - return self.state_territory + return self.portfolio.get_state_territory_display() + return self.get_state_territory_display() @property def converted_zipcode(self): if self.portfolio: - return self.portfolio.zipcode - return self.zipcode + return self.portfolio.display_zipcode + return self.display_zipcode @property def converted_urbanization(self): if self.portfolio: - return self.portfolio.urbanization - return self.urbanization + return self.portfolio.display_urbanization + return self.display_urbanization + + + # ----- Portfolio Properties (display values)----- + @property + def converted_generic_org_type_display(self): + if self.portfolio: + return self.portfolio.get_organization_type_display() + return self.get_generic_org_type_display() \ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index e493c8715..1d6bee4b6 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -539,12 +539,14 @@ def get_computed_fields(cls, delimiter=", ", **kwargs): # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__organization_type")), # Otherwise, return the natively assigned value - default=F("organization_type"), + default=F("generic_org_type"), output_field=CharField(), ), "converted_federal_agency": Case( # When portfolio is present, use its value instead - When(portfolio__isnull=False, then=F("portfolio__federal_agency__agency")), + When( + Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), + then=F("portfolio__federal_agency__agency")), # Otherwise, return the natively assigned value default=F("federal_agency__agency"), output_field=CharField(), @@ -1628,12 +1630,15 @@ def get_computed_fields(cls, delimiter=", ", **kwargs): # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__organization_type")), # Otherwise, return the natively assigned value - default=F("organization_type"), + default=F("generic_org_type"), output_field=CharField(), ), "converted_federal_agency": Case( # When portfolio is present, use its value instead - When(portfolio__isnull=False, then=F("portfolio__federal_agency__agency")), + When( + Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), + then=F("portfolio__federal_agency__agency") + ), # Otherwise, return the natively assigned value default=F("federal_agency__agency"), output_field=CharField(), From fb2fad67f9a64d8cb3d4c789c1c58483b1c7812a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Dec 2024 01:17:18 -0700 Subject: [PATCH 22/29] Efficiency updates and sorting fixes --- src/registrar/admin.py | 150 ++++++++++++--------- src/registrar/models/domain_information.py | 14 +- src/registrar/models/domain_request.py | 14 ++ src/registrar/utility/csv_export.py | 34 +++-- 4 files changed, 138 insertions(+), 74 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8f345814d..380b8e839 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1503,7 +1503,7 @@ def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( Q(portfolio__organization_type=self.value()) | - Q(generic_org_type=self.value()) + Q(portfolio__isnull=True, generic_org_type=self.value()) ) return queryset @@ -1514,7 +1514,7 @@ def queryset(self, request, queryset): # Customize column header text @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): - return obj.converted_generic_org_type + return obj.converted_generic_org_type_display # Columns list_display = [ @@ -1667,7 +1667,6 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): 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): @@ -1709,9 +1708,9 @@ def lookups(self, request, model_admin): converted_generic_orgs = set() # Populate the set with tuples of (value, display value) - for domain_info in DomainInformation.objects.all(): - converted_generic_org = domain_info.converted_generic_org_type # Actual value - converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value + for domain_request in DomainRequest.objects.all(): + converted_generic_org = domain_request.converted_generic_org_type # Actual value + converted_generic_org_display = domain_request.converted_generic_org_type_display # Display value if converted_generic_org: converted_generic_orgs.add( @@ -1726,7 +1725,7 @@ def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( Q(portfolio__organization_type=self.value()) | - Q(generic_org_type=self.value()) + Q(portfolio__isnull=True, generic_org_type=self.value()) ) return queryset @@ -1741,20 +1740,30 @@ class FederalTypeFilter(admin.SimpleListFilter): def lookups(self, request, model_admin): converted_federal_types = set() + # Populate the set with tuples of (value, display value) for domain_request in DomainRequest.objects.all(): - converted_federal_type = domain_request.converted_federal_type + converted_federal_type = domain_request.converted_federal_type # Actual value + converted_federal_type_display = domain_request.converted_federal_type_display # Display value + if converted_federal_type: - converted_federal_types.add(converted_federal_type) + converted_federal_types.add( + (converted_federal_type, converted_federal_type_display) # Value, Display + ) - return sorted((type, type) for type in converted_federal_types) + # Sort the set by display value + return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value # Filter queryset def queryset(self, request, queryset): if self.value(): # Check if a federal type is selected in the filter return queryset.filter( - Q(portfolio__federal_type=self.value()) | - Q(federal_type=self.value()) + Q(portfolio__federal_agency__federal_type=self.value()) | + Q(portfolio__isnull=True, federal_type=self.value()) ) + # return queryset.filter( + # Q(portfolio__federal_type=self.value()) | + # Q(portfolio__isnull=True, federal_type=self.value()) + # ) return queryset class InvestigatorFilter(admin.SimpleListFilter): @@ -1819,7 +1828,7 @@ def queryset(self, request, queryset): @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): - return obj.converted_generic_org_type + return obj.converted_generic_org_type_display @admin.display(description=_("Organization Name")) def converted_organization_name(self, obj): @@ -1831,7 +1840,7 @@ def converted_federal_agency(self, obj): @admin.display(description=_("Federal Type")) def converted_federal_type(self, obj): - return obj.converted_federal_type + return obj.converted_federal_type_display @admin.display(description=_("City")) def converted_city(self, obj): @@ -2768,8 +2777,53 @@ def lookups(self, request, model_admin): # Filter queryset def queryset(self, request, queryset): + if self.value(): # Check if a generic org is selected in the filter + + # return queryset.filter(converted_generic_org_type = self.value) + return queryset.filter( + Q(domain_info__portfolio__organization_type=self.value()) | + Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) + ) - annotated_queryset = queryset.annotate( + return queryset + + + + class FederalTypeFilter(admin.SimpleListFilter): + """Custom Federal Type filter that accomodates portfolio feature. + If we have a portfolio, use the portfolio's federal type. If not, use the + federal type in the Domain Information object.""" + + title = "federal type" + parameter_name = "converted_federal_types" + + def lookups(self, request, model_admin): + converted_federal_types = set() + + # Populate the set with tuples of (value, display value) + for domain_info in DomainInformation.objects.all(): + converted_federal_type = domain_info.converted_federal_type # Actual value + converted_federal_type_display = domain_info.converted_federal_type_display # Display value + + if converted_federal_type: + converted_federal_types.add( + (converted_federal_type, converted_federal_type_display) # Value, Display + ) + + # Sort the set by display value + return sorted(converted_federal_types, key=lambda x: x[1]) # x[1] is the display value + + # Filter queryset + def queryset(self, request, queryset): + if self.value(): # Check if a federal type is selected in the filter + return queryset.filter( + Q(domain_info__portfolio__federal_agency__federal_type=self.value()) | + Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value()) + ) + return queryset + + def get_annotated_queryset(self, queryset): + return queryset.annotate( converted_generic_org_type=Case( # When portfolio is present, use its value instead When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_type")), @@ -2785,6 +2839,15 @@ def queryset(self, request, queryset): # Otherwise, return the natively assigned value default=F("domain_info__federal_agency__agency"), ), + converted_federal_type=Case( + # When portfolio is present, use its value instead + When( + Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), + then=F("domain_info__portfolio__federal_agency__federal_type") + ), + # Otherwise, return the natively assigned value + default=F("domain_info__federal_agency__federal_type"), + ), converted_organization_name=Case( # When portfolio is present, use its value instead When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_name")), @@ -2805,44 +2868,6 @@ def queryset(self, request, queryset): ), ) - if self.value(): # Check if a generic org is selected in the filter - return annotated_queryset.filter( - Q(domain_info__portfolio__organization_type=self.value()) | - Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) - ) - - return annotated_queryset - - - - class FederalTypeFilter(admin.SimpleListFilter): - """Custom Federal Type filter that accomodates portfolio feature. - If we have a portfolio, use the portfolio's federal type. If not, use the - federal type in the Domain Information object.""" - - title = "federal type" - parameter_name = "converted_federal_types" - - def lookups(self, request, model_admin): - converted_federal_types = set() - # converted_federal_types.add("blah") - - for domainInfo in DomainInformation.objects.all(): - converted_federal_type = domainInfo.converted_federal_type - if converted_federal_type: - converted_federal_types.add(converted_federal_type) - - return sorted((fed, fed) for fed in converted_federal_types) - - # Filter queryset - def queryset(self, request, queryset): - if self.value(): # Check if a federal type is selected in the filter - return queryset.filter( - Q(portfolio__federal_type=self.value()) | - Q(federal_type=self.value()) - ) - return queryset - # Filters list_filter = [GenericOrgFilter, FederalTypeFilter, ElectionOfficeFilter, "state"] @@ -2855,7 +2880,7 @@ def queryset(self, request, queryset): list_display = [ "name", "converted_generic_org_type", - "federal_type", + "converted_federal_type", "converted_federal_agency", "converted_organization_name", "custom_election_board", @@ -2883,7 +2908,7 @@ def queryset(self, request, queryset): def converted_generic_org_type(self, obj): return obj.domain_info.converted_generic_org_type_display - converted_generic_org_type.admin_order_field = "domain_info__converted_generic_org_type" # type: ignore + converted_generic_org_type.admin_order_field = "converted_generic_org_type" # type: ignore # Use native value for the change form def generic_org_type(self, obj): @@ -2894,7 +2919,7 @@ def generic_org_type(self, obj): def converted_federal_agency(self, obj): return obj.domain_info.converted_federal_agency - converted_federal_agency.admin_order_field = "domain_info__converted_federal_agency" # type: ignore + converted_federal_agency.admin_order_field = "converted_federal_agency" # type: ignore # Use native value for the change form def federal_agency(self, obj): @@ -2907,9 +2932,9 @@ def federal_agency(self, obj): # Use converted value in the table @admin.display(description=_("Federal Type")) def converted_federal_type(self, obj): - return obj.domain_info.converted_federal_type + return obj.domain_info.converted_federal_type_display - converted_federal_type.admin_order_field = "domain_info__converted_federal_type" # type: ignore + converted_federal_type.admin_order_field = "converted_federal_type" # type: ignore # Use native value for the change form def federal_type(self, obj): @@ -2921,7 +2946,7 @@ def federal_type(self, obj): def converted_organization_name(self, obj): return obj.domain_info.converted_organization_name - converted_organization_name.admin_order_field = "domain_info__converted_organization_name" # type: ignore + converted_organization_name.admin_order_field = "converted_organization_name" # type: ignore # Use native value for the change form def organization_name(self, obj): @@ -2933,7 +2958,7 @@ def organization_name(self, obj): def converted_city(self, obj): return obj.domain_info.converted_city - converted_city.admin_order_field = "domain_info__converted_city" # type: ignore + converted_city.admin_order_field = "converted_city" # type: ignore # Use native value for the change form def city(self, obj): @@ -2945,7 +2970,7 @@ def city(self, obj): def converted_state_territory(self, obj): return obj.domain_info.converted_state_territory - converted_state_territory.admin_order_field = "domain_info__converted_state_territory" # type: ignore + converted_state_territory.admin_order_field = "converted_state_territory" # type: ignore # Use native value for the change form def state_territory(self, obj): @@ -3254,7 +3279,8 @@ def has_change_permission(self, request, obj=None): def get_queryset(self, request): """Custom get_queryset to filter by portfolio if portfolio is in the request params.""" - qs = super().get_queryset(request) + initial_qs = super().get_queryset(request) + qs = self.get_annotated_queryset(initial_qs) # Check if a 'portfolio' parameter is passed in the request portfolio_id = request.GET.get("portfolio") if portfolio_id: diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 60c4d8c75..3fb7e3e8d 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -444,14 +444,14 @@ def converted_generic_org_type(self): @property def converted_federal_agency(self): if self.portfolio: - return self.portfolio.federal_agency - return self.federal_agency + return self.portfolio.federal_agency.agency + return self.federal_agency.agency @property def converted_federal_type(self): if self.portfolio: return self.portfolio.federal_type - return self.get_federal_type_display() + return self.federal_type @property def converted_senior_official(self): @@ -501,4 +501,10 @@ def converted_urbanization(self): def converted_generic_org_type_display(self): if self.portfolio: return self.portfolio.get_organization_type_display() - return self.get_generic_org_type_display() \ No newline at end of file + return self.get_generic_org_type_display() + + @property + def converted_federal_type_display(self): + if self.portfolio: + return self.portfolio.federal_agency.get_federal_type_display() + return self.get_federal_type_display() \ No newline at end of file diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index eecc6e3c1..780bd8719 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1478,3 +1478,17 @@ def converted_senior_official(self): if self.portfolio: return self.portfolio.senior_official return self.senior_official + + + # ----- Portfolio Properties (display values)----- + @property + def converted_generic_org_type_display(self): + if self.portfolio: + return self.portfolio.get_organization_type_display() + return self.get_generic_org_type_display() + + @property + def converted_federal_type_display(self): + if self.portfolio: + return self.portfolio.federal_agency.get_federal_type_display() + return self.get_federal_type_display() \ No newline at end of file diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 1d6bee4b6..24cbd272c 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -767,14 +767,21 @@ def parse_row(cls, columns, model): def get_filtered_domain_infos_by_org(domain_infos_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value.""" - return domain_infos_to_filter.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domainInfos.id - for domainInfos in domain_infos_to_filter - if domainInfos.converted_generic_org_type and domainInfos.converted_generic_org_type == org_to_filter_by - ] - ) + + annotated_queryset = domain_infos_to_filter.annotate( + converted_generic_org_type=Case( + # Recreate the logic of the converted_generic_org_type property + # here in annotations + When( + portfolio__isnull=False, + then=F("portfolio__organization_type") + ), + default=F("organization_type"), + output_field=CharField(), + ) + ) + return annotated_queryset.filter(converted_generic_org_type=org_to_filter_by) + @classmethod def get_sliced_domains(cls, filter_condition): @@ -1654,6 +1661,17 @@ def get_computed_fields(cls, delimiter=", ", **kwargs): default=F("federal_type"), output_field=CharField(), ), + "converted_federal_type": Case( + # When portfolio is present, use its value instead + # NOTE: this is an @Property funciton in portfolio. + When( + Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), + then=F("portfolio__federal_agency__federal_type"), + ), + # Otherwise, return the natively assigned value + default=F("federal_type"), + output_field=CharField(), + ), "converted_organization_name": Case( # When portfolio is present, use its value instead When(portfolio__isnull=False, then=F("portfolio__organization_name")), From 3b9e9e39eb4c7135bf399e12641e3c1f10b7f6cf Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Dec 2024 14:11:40 -0700 Subject: [PATCH 23/29] remove dead code (missed in another merge) --- src/registrar/views/portfolios.py | 33 +------------------------------ 1 file changed, 1 insertion(+), 32 deletions(-) diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 89fae0862..429934dcf 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -625,35 +625,4 @@ def submit_new_member(self, form): else: if permission_exists: messages.warning(self.request, "User is already a member of this portfolio.") - return redirect(self.get_success_url()) - - # look up a user with that email - try: - requested_user = User.objects.get(email=requested_email) - except User.DoesNotExist: - # no matching user, go make an invitation - return self._make_invitation(requested_email, requestor) - else: - # If user already exists, check to see if they are part of the portfolio already - # If they are already part of the portfolio, raise an error. Otherwise, send an invite. - existing_user = UserPortfolioPermission.objects.get(user=requested_user, portfolio=self.object) - if existing_user: - messages.warning(self.request, "User is already a member of this portfolio.") - else: - try: - self._send_portfolio_invitation_email(requested_email, requestor, add_success=False) - except EmailSendingError: - logger.warn( - "Could not send email invitation (EmailSendingError)", - self.object, - exc_info=True, - ) - messages.warning(self.request, "Could not send email invitation.") - except Exception: - logger.warn( - "Could not send email invitation (Other Exception)", - self.object, - exc_info=True, - ) - messages.warning(self.request, "Could not send email invitation.") - return redirect(self.get_success_url()) + return redirect(self.get_success_url()) \ No newline at end of file From 87a9655828164182542320d8338eecac05ba91dc Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Dec 2024 19:43:39 -0700 Subject: [PATCH 24/29] Cleanup and unit test fixes --- src/registrar/admin.py | 6 -- src/registrar/tests/common.py | 3 +- src/registrar/tests/test_admin_domain.py | 9 ++- src/registrar/tests/test_reports.py | 99 ++++++++++++------------ src/registrar/utility/csv_export.py | 43 ++++++---- 5 files changed, 87 insertions(+), 73 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 380b8e839..8e918e442 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1760,10 +1760,6 @@ def queryset(self, request, queryset): Q(portfolio__federal_agency__federal_type=self.value()) | Q(portfolio__isnull=True, federal_type=self.value()) ) - # return queryset.filter( - # Q(portfolio__federal_type=self.value()) | - # Q(portfolio__isnull=True, federal_type=self.value()) - # ) return queryset class InvestigatorFilter(admin.SimpleListFilter): @@ -2778,8 +2774,6 @@ def lookups(self, request, model_admin): # Filter queryset def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter - - # return queryset.filter(converted_generic_org_type = self.value) return queryset.filter( Q(domain_info__portfolio__organization_type=self.value()) | Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 6a5bbdd78..055cd39f7 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -563,9 +563,10 @@ def sharedSetUp(cls): cls.federal_agency_1, _ = FederalAgency.objects.get_or_create(agency="World War I Centennial Commission") cls.federal_agency_2, _ = FederalAgency.objects.get_or_create(agency="Armed Forces Retirement Home") + cls.federal_agency_3, _ = FederalAgency.objects.get_or_create(agency="Portfolio 1 Federal Agency",federal_type="executive") cls.portfolio_1, _ = Portfolio.objects.get_or_create( - creator=cls.custom_superuser, federal_agency=cls.federal_agency_1 + creator=cls.custom_superuser, federal_agency=cls.federal_agency_3, organization_type="federal" ) current_date = get_time_aware_date(datetime(2024, 4, 2)) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index fb1511e1d..26f675150 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -30,6 +30,9 @@ from unittest.mock import ANY, call, patch import boto3_mocking # type: ignore +import logging + +logger = logging.getLogger(__name__) class TestDomainAdminAsStaff(MockEppLib): @@ -723,9 +726,9 @@ def test_short_org_name_in_domains_list(self): domain_request.approve() response = self.client.get("/admin/registrar/domain/") - # The total count should reflect the fact that we are pulling from portfolio - # data when portfolios are present - self.assertContains(response, "Federal", count=98) + # There are 4 template references to Federal (4) plus four references in the table + # for our actual domain_request + self.assertContains(response, "Federal", count=57) # This may be a bit more robust self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 846fa5915..305340a2a 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -53,6 +53,12 @@ from django.contrib.admin.models import LogEntry, ADDITION from django.contrib.contenttypes.models import ContentType +# ---Logger +import logging +from venv import logger +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +logger = logging.getLogger(__name__) + class CsvReportsTest(MockDbForSharedTests): """Tests to determine if we are uploading our reports correctly.""" @@ -71,8 +77,8 @@ def test_generate_federal_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), + call("cdomain1.gov,Federal - Executive,Portfolio 1 Federal Agency,,,,(blank)\r\n"), call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), ] @@ -93,8 +99,8 @@ def test_generate_full_report(self): fake_open = mock_open() expected_file_content = [ call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"), + call("cdomain1.gov,Federal - Executive,Portfolio 1 Federal Agency,,,,(blank)\r\n"), call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), - call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"), call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"), call("zdomain12.gov,Interstate,,,,,(blank)\r\n"), @@ -251,25 +257,23 @@ def test_domain_data_type(self): # We expect READY domains, # sorted alphabetially by domain name expected_content = ( - "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO," - "SO email,Security contact email,Domain managers,Invited domain managers\n" - "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,WorldWarICentennialCommission" - ",,,, ,,(blank),meoward@rocks.com,\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,WorldWarICentennialCommission" - ",,,, ,,(blank)," + "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO,SO email," + "Security contact email,Domain managers,Invited domain managers\n" + "adomain2.gov,Dns needed,(blank),(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," + "meoward@rocks.com,squeaker@rocks.com\n" + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' - "adomain10.gov,Ready,2024-04-03,(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),," - "squeaker@rocks.com\n" - "bdomain4.gov,Unknown,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "bdomain5.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "bdomain6.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "ddomain3.gov,On hold,(blank),2023-11-15,Federal,ArmedForcesRetirementHome,,,, ,," - "security@mail.gov,,\n" - "sdomain8.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "xdomain7.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "zdomain9.gov,Deleted,(blank),(blank),Federal,ArmedForcesRetirementHome,,,, ,,(blank),,\n" - "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank),meoward@rocks.com," + "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,World War I Centennial Commission,,,, ,,(blank)," + "meoward@rocks.com,\n" + "adomain10.gov,Ready,2024-04-03,(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),," "squeaker@rocks.com\n" + "bdomain4.gov,Unknown,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" + "bdomain5.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" + "bdomain6.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" + "ddomain3.gov,On hold,(blank),2023-11-15,Federal,Armed Forces Retirement Home,,,, ,,security@mail.gov,,\n" + "sdomain8.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" + "xdomain7.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" + "zdomain9.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,\n" ) @@ -313,13 +317,11 @@ def test_domain_data_type_user(self): # We expect only domains associated with the user expected_content = ( "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name," - "City,State,SO,SO email," - "Security contact email,Domain managers,Invited domain managers\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive," - "WorldWarICentennialCommission,,,, ,,(blank)," - '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' - "adomain2.gov,Dns needed,(blank),(blank),Interstate,,,,, ,,(blank)," + "City,State,SO,SO email,Security contact email,Domain managers,Invited domain managers\n" + "adomain2.gov,Dns needed,(blank),(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," '"info@example.com, meoward@rocks.com",squeaker@rocks.com\n' + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," + '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' ) # Normalize line endings and remove commas, @@ -494,8 +496,8 @@ def test_domain_data_full(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" + "defaultsecurity.gov,Federal - Executive,Portfolio1FederalAgency,,,,(blank)\n" "cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" "adomain10.gov,Federal,ArmedForcesRetirementHome,,,,(blank)\n" "ddomain3.gov,Federal,ArmedForcesRetirementHome,,,,security@mail.gov\n" "zdomain12.gov,Interstate,,,,,(blank)\n" @@ -535,8 +537,8 @@ def test_domain_data_federal(self): # sorted alphabetially by domain name expected_content = ( "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n" + "defaultsecurity.gov,Federal - Executive,Portfolio1FederalAgency,,,,(blank)\n" "cdomain11.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" - "defaultsecurity.gov,Federal - Executive,WorldWarICentennialCommission,,,,(blank)\n" "adomain10.gov,Federal,ArmedForcesRetirementHome,,,,(blank)\n" "ddomain3.gov,Federal,ArmedForcesRetirementHome,,,,security@mail.gov\n" ) @@ -591,7 +593,7 @@ def test_domain_growth(self): expected_content = ( "Domain name,Domain type,Agency,Organization name,City," "State,Status,Expiration date, Deleted\n" - "cdomain1.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank)\n" + "cdomain1.gov,Federal-Executive,Portfolio1FederalAgency,Ready,(blank)\n" "adomain10.gov,Federal,ArmedForcesRetirementHome,Ready,(blank)\n" "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank)\n" "zdomain12.gov,Interstate,Ready,(blank)\n" @@ -726,8 +728,9 @@ def test_domain_request_growth(self): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - @less_console_noise_decorator + # @less_console_noise_decorator def test_domain_request_data_full(self): + self.maxDiff = None """Tests the full domain request report.""" # Remove "Submitted at" because we can't guess this immutable, dynamically generated test data columns = [ @@ -768,35 +771,31 @@ def test_domain_request_data_full(self): csv_file.seek(0) # Read the content into a variable csv_content = csv_file.read() + 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,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," + "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," + "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," + "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,,Testorg,N/A,,NY,2,,,,0,1,city1.gov,Testy,Tester," - "testy@town.com," - "Chief Tester,Purpose of the site,There is more,Testy Tester testy2@town.com,,city.com,\n" - 'city3.gov,Submitted,Federal,Executive,,Testorg,N/A,,NY,2,,,,0,1,"cheeseville.gov, city1.gov,' - 'igorville.gov",Testy,Tester,testy@town.com,Chief Tester,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' + "city2.gov,In review,Federal,Executive,Portfolio 1 Federal Agency,,N/A,,,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,,,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,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,,Testorg,N/A,,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," + "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,,,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" ) + # Normalize line endings and remove commas, # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 24cbd272c..025335afb 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -138,7 +138,7 @@ def get_filter_conditions(cls, **kwargs): return Q() @classmethod - def get_computed_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, **kwargs): """ Get a dict of computed fields. These are fields that do not exist on the model normally and will be passed to .annotate() when building a queryset. @@ -526,7 +526,7 @@ def model(cls): return DomainInformation @classmethod - def get_computed_fields(cls, delimiter=", ", **kwargs): + def get_computed_fields(cls, **kwargs): """ Get a dict of computed fields. """ @@ -776,7 +776,7 @@ def get_filtered_domain_infos_by_org(domain_infos_to_filter, org_to_filter_by): portfolio__isnull=False, then=F("portfolio__organization_type") ), - default=F("organization_type"), + default=F("generic_org_type"), output_field=CharField(), ) ) @@ -880,7 +880,7 @@ def get_columns(cls): ] @classmethod - def get_annotations_for_sort(cls, delimiter=", "): + def get_annotations_for_sort(cls): """ Get a dict of annotations to make available for sorting. """ @@ -1613,15 +1613,32 @@ def model(cls): def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value""" - return domain_requests_to_filter.filter( - # Filter based on the generic org value returned by converted_generic_org_type - id__in=[ - domainRequest.id - for domainRequest in domain_requests_to_filter - if domainRequest.converted_generic_org_type - and domainRequest.converted_generic_org_type == org_to_filter_by - ] - ) + annotated_queryset = domain_requests_to_filter.annotate( + converted_generic_org_type=Case( + # Recreate the logic of the converted_generic_org_type property + # here in annotations + When( + portfolio__isnull=False, + then=F("portfolio__organization_type") + ), + default=F("generic_org_type"), + output_field=CharField(), + ) + ) + return annotated_queryset.filter(converted_generic_org_type=org_to_filter_by) + + # return domain_requests_to_filter.filter( + # # Filter based on the generic org value returned by converted_generic_org_type + # id__in=[ + # domainRequest.id + # for domainRequest in domain_requests_to_filter + # if domainRequest.converted_generic_org_type + # and domainRequest.converted_generic_org_type == org_to_filter_by + # ] + # ) + + + @classmethod def get_computed_fields(cls, delimiter=", ", **kwargs): From d5f799347a580d697e38341447e1483ac24c8a14 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Dec 2024 19:46:43 -0700 Subject: [PATCH 25/29] cleanup --- src/registrar/utility/csv_export.py | 38 ++++++++++++++--------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 025335afb..ed314812e 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -213,6 +213,25 @@ def annotate_and_retrieve_fields( return cls.update_queryset(queryset, **kwargs) + @classmethod + def export_data_to_csv(cls, csv_file, **kwargs): + """ + All domain metadata: + Exports domains of all statuses plus domain managers. + """ + writer = csv.writer(csv_file) + columns = cls.get_columns() + models_dict = cls.get_model_annotation_dict(**kwargs) + + # Write to csv file before the write_csv + cls.write_csv_before(writer, **kwargs) + + # Write the csv file + rows = cls.write_csv(writer, columns, models_dict) + + # Return rows that for easier parsing and testing + return rows + @classmethod def get_annotated_queryset(cls, **kwargs): """Returns an annotated queryset based off of all query conditions.""" @@ -243,25 +262,6 @@ def get_annotated_queryset(cls, **kwargs): @classmethod def get_model_annotation_dict(cls, **kwargs): return convert_queryset_to_dict(cls.get_annotated_queryset(**kwargs), is_model=False) - - @classmethod - def export_data_to_csv(cls, csv_file, **kwargs): - """ - All domain metadata: - Exports domains of all statuses plus domain managers. - """ - writer = csv.writer(csv_file) - columns = cls.get_columns() - models_dict = cls.get_model_annotation_dict(**kwargs) - - # Write to csv file before the write_csv - cls.write_csv_before(writer, **kwargs) - - # Write the csv file - rows = cls.write_csv(writer, columns, models_dict) - - # Return rows that for easier parsing and testing - return rows @classmethod def write_csv( From 958d6dc35ed940d0ea743c0a37c9773375a2bf71 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 4 Dec 2024 19:52:56 -0700 Subject: [PATCH 26/29] linted --- src/registrar/admin.py | 142 ++++++++++----------- src/registrar/models/domain_information.py | 8 +- src/registrar/models/domain_request.py | 5 +- src/registrar/tests/common.py | 4 +- src/registrar/tests/test_reports.py | 32 ++--- src/registrar/utility/csv_export.py | 60 +++------ src/registrar/views/portfolios.py | 2 +- 7 files changed, 112 insertions(+), 141 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8e918e442..5ae3457c3 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -51,7 +51,7 @@ from django.contrib.auth.forms import UserChangeForm, UsernameField from django.contrib.admin.views.main import IGNORED_PARAMS from django_admin_multiple_choice_list_filter.list_filters import MultipleChoiceListFilter -from import_export import resources, fields +from import_export import resources from import_export.admin import ImportExportModelAdmin from django.core.exceptions import ObjectDoesNotExist from django.contrib.admin.widgets import FilteredSelectMultiple @@ -1489,11 +1489,9 @@ def lookups(self, request, model_admin): for domain_info in DomainInformation.objects.all(): converted_generic_org = domain_info.converted_generic_org_type # Actual value converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value - + if converted_generic_org: - converted_generic_orgs.add( - (converted_generic_org, converted_generic_org_display) # Value, Display - ) + converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display # Sort the set by display value return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value @@ -1502,9 +1500,9 @@ def lookups(self, request, model_admin): def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( - Q(portfolio__organization_type=self.value()) | - Q(portfolio__isnull=True, generic_org_type=self.value()) - ) + Q(portfolio__organization_type=self.value()) + | Q(portfolio__isnull=True, generic_org_type=self.value()) + ) return queryset resource_classes = [DomainInformationResource] @@ -1666,7 +1664,6 @@ def formfield_for_foreignkey(self, db_field, request, **kwargs): # 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): @@ -1711,11 +1708,9 @@ def lookups(self, request, model_admin): for domain_request in DomainRequest.objects.all(): converted_generic_org = domain_request.converted_generic_org_type # Actual value converted_generic_org_display = domain_request.converted_generic_org_type_display # Display value - + if converted_generic_org: - converted_generic_orgs.add( - (converted_generic_org, converted_generic_org_display) # Value, Display - ) + converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display # Sort the set by display value return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value @@ -1724,9 +1719,9 @@ def lookups(self, request, model_admin): def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( - Q(portfolio__organization_type=self.value()) | - Q(portfolio__isnull=True, generic_org_type=self.value()) - ) + Q(portfolio__organization_type=self.value()) + | Q(portfolio__isnull=True, generic_org_type=self.value()) + ) return queryset class FederalTypeFilter(admin.SimpleListFilter): @@ -1744,7 +1739,7 @@ def lookups(self, request, model_admin): for domain_request in DomainRequest.objects.all(): converted_federal_type = domain_request.converted_federal_type # Actual value converted_federal_type_display = domain_request.converted_federal_type_display # Display value - + if converted_federal_type: converted_federal_types.add( (converted_federal_type, converted_federal_type_display) # Value, Display @@ -1757,9 +1752,9 @@ def lookups(self, request, model_admin): def queryset(self, request, queryset): if self.value(): # Check if a federal type is selected in the filter return queryset.filter( - Q(portfolio__federal_agency__federal_type=self.value()) | - Q(portfolio__isnull=True, federal_type=self.value()) - ) + Q(portfolio__federal_agency__federal_type=self.value()) + | Q(portfolio__isnull=True, federal_type=self.value()) + ) return queryset class InvestigatorFilter(admin.SimpleListFilter): @@ -2761,27 +2756,22 @@ def lookups(self, request, model_admin): for domain_info in DomainInformation.objects.all(): converted_generic_org = domain_info.converted_generic_org_type # Actual value converted_generic_org_display = domain_info.converted_generic_org_type_display # Display value - + if converted_generic_org: - converted_generic_orgs.add( - (converted_generic_org, converted_generic_org_display) # Value, Display - ) + converted_generic_orgs.add((converted_generic_org, converted_generic_org_display)) # Value, Display # Sort the set by display value return sorted(converted_generic_orgs, key=lambda x: x[1]) # x[1] is the display value - # Filter queryset def queryset(self, request, queryset): if self.value(): # Check if a generic org is selected in the filter return queryset.filter( - Q(domain_info__portfolio__organization_type=self.value()) | - Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) - ) - + Q(domain_info__portfolio__organization_type=self.value()) + | Q(domain_info__portfolio__isnull=True, domain_info__generic_org_type=self.value()) + ) + return queryset - - class FederalTypeFilter(admin.SimpleListFilter): """Custom Federal Type filter that accomodates portfolio feature. @@ -2798,7 +2788,7 @@ def lookups(self, request, model_admin): for domain_info in DomainInformation.objects.all(): converted_federal_type = domain_info.converted_federal_type # Actual value converted_federal_type_display = domain_info.converted_federal_type_display # Display value - + if converted_federal_type: converted_federal_types.add( (converted_federal_type, converted_federal_type_display) # Value, Display @@ -2811,56 +2801,56 @@ def lookups(self, request, model_admin): def queryset(self, request, queryset): if self.value(): # Check if a federal type is selected in the filter return queryset.filter( - Q(domain_info__portfolio__federal_agency__federal_type=self.value()) | - Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value()) - ) + Q(domain_info__portfolio__federal_agency__federal_type=self.value()) + | Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value()) + ) return queryset def get_annotated_queryset(self, queryset): return queryset.annotate( - converted_generic_org_type=Case( - # When portfolio is present, use its value instead - When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_type")), - # Otherwise, return the natively assigned value - default=F("domain_info__generic_org_type"), - ), - converted_federal_agency=Case( - # When portfolio is present, use its value instead - When( - Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), - then=F("domain_info__portfolio__federal_agency__agency") - ), - # Otherwise, return the natively assigned value - default=F("domain_info__federal_agency__agency"), - ), - converted_federal_type=Case( - # When portfolio is present, use its value instead - When( - Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), - then=F("domain_info__portfolio__federal_agency__federal_type") - ), - # Otherwise, return the natively assigned value - default=F("domain_info__federal_agency__federal_type"), - ), - converted_organization_name=Case( - # When portfolio is present, use its value instead - When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_name")), - # Otherwise, return the natively assigned value - default=F("domain_info__organization_name"), - ), - converted_city=Case( - # When portfolio is present, use its value instead - When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__city")), - # Otherwise, return the natively assigned value - default=F("domain_info__city"), + converted_generic_org_type=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_type")), + # Otherwise, return the natively assigned value + default=F("domain_info__generic_org_type"), + ), + converted_federal_agency=Case( + # When portfolio is present, use its value instead + When( + Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), + then=F("domain_info__portfolio__federal_agency__agency"), ), - converted_state_territory=Case( - # When portfolio is present, use its value instead - When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__state_territory")), - # Otherwise, return the natively assigned value - default=F("domain_info__state_territory"), + # Otherwise, return the natively assigned value + default=F("domain_info__federal_agency__agency"), + ), + converted_federal_type=Case( + # When portfolio is present, use its value instead + When( + Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False), + then=F("domain_info__portfolio__federal_agency__federal_type"), ), - ) + # Otherwise, return the natively assigned value + default=F("domain_info__federal_agency__federal_type"), + ), + converted_organization_name=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__organization_name")), + # Otherwise, return the natively assigned value + default=F("domain_info__organization_name"), + ), + converted_city=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__city")), + # Otherwise, return the natively assigned value + default=F("domain_info__city"), + ), + converted_state_territory=Case( + # When portfolio is present, use its value instead + When(domain_info__portfolio__isnull=False, then=F("domain_info__portfolio__state_territory")), + # Otherwise, return the natively assigned value + default=F("domain_info__state_territory"), + ), + ) # Filters list_filter = [GenericOrgFilter, FederalTypeFilter, ElectionOfficeFilter, "state"] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 3fb7e3e8d..378d59137 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -426,7 +426,6 @@ def get_state_display_of_domain(self): else: return None - # ----- Portfolio Properties ----- @property @@ -434,7 +433,7 @@ def converted_organization_name(self): if self.portfolio: return self.portfolio.organization_name return self.organization_name - + @property def converted_generic_org_type(self): if self.portfolio: @@ -495,16 +494,15 @@ def converted_urbanization(self): return self.portfolio.display_urbanization return self.display_urbanization - # ----- Portfolio Properties (display values)----- @property def converted_generic_org_type_display(self): if self.portfolio: return self.portfolio.get_organization_type_display() return self.get_generic_org_type_display() - + @property def converted_federal_type_display(self): if self.portfolio: return self.portfolio.federal_agency.get_federal_type_display() - return self.get_federal_type_display() \ No newline at end of file + return self.get_federal_type_display() diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 780bd8719..3fa889a3b 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1479,16 +1479,15 @@ def converted_senior_official(self): return self.portfolio.senior_official return self.senior_official - # ----- Portfolio Properties (display values)----- @property def converted_generic_org_type_display(self): if self.portfolio: return self.portfolio.get_organization_type_display() return self.get_generic_org_type_display() - + @property def converted_federal_type_display(self): if self.portfolio: return self.portfolio.federal_agency.get_federal_type_display() - return self.get_federal_type_display() \ No newline at end of file + return self.get_federal_type_display() diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 055cd39f7..e1f4f5a27 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -563,7 +563,9 @@ def sharedSetUp(cls): cls.federal_agency_1, _ = FederalAgency.objects.get_or_create(agency="World War I Centennial Commission") cls.federal_agency_2, _ = FederalAgency.objects.get_or_create(agency="Armed Forces Retirement Home") - cls.federal_agency_3, _ = FederalAgency.objects.get_or_create(agency="Portfolio 1 Federal Agency",federal_type="executive") + cls.federal_agency_3, _ = FederalAgency.objects.get_or_create( + agency="Portfolio 1 Federal Agency", federal_type="executive" + ) cls.portfolio_1, _ = Portfolio.objects.get_or_create( creator=cls.custom_superuser, federal_agency=cls.federal_agency_3, organization_type="federal" diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 305340a2a..888abe302 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -53,12 +53,6 @@ from django.contrib.admin.models import LogEntry, ADDITION from django.contrib.contenttypes.models import ContentType -# ---Logger -import logging -from venv import logger -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper -logger = logging.getLogger(__name__) - class CsvReportsTest(MockDbForSharedTests): """Tests to determine if we are uploading our reports correctly.""" @@ -257,20 +251,25 @@ def test_domain_data_type(self): # We expect READY domains, # sorted alphabetially by domain name expected_content = ( - "Domain name,Status,First ready on,Expiration date,Domain type,Agency,Organization name,City,State,SO,SO email," + "Domain name,Status,First ready on,Expiration date,Domain type,Agency," + "Organization name,City,State,SO,SO email," "Security contact email,Domain managers,Invited domain managers\n" - "adomain2.gov,Dns needed,(blank),(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," + "adomain2.gov,Dns needed,(blank),(blank),Federal - Executive," + "Portfolio 1 Federal Agency,,,, ,,(blank)," "meoward@rocks.com,squeaker@rocks.com\n" - "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,Portfolio 1 Federal Agency,,,, ,,(blank)," + "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive," + "Portfolio 1 Federal Agency,,,, ,,(blank)," '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n' - "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,World War I Centennial Commission,,,, ,,(blank)," + "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive," + "World War I Centennial Commission,,,, ,,(blank)," "meoward@rocks.com,\n" "adomain10.gov,Ready,2024-04-03,(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),," "squeaker@rocks.com\n" "bdomain4.gov,Unknown,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" "bdomain5.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" "bdomain6.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" - "ddomain3.gov,On hold,(blank),2023-11-15,Federal,Armed Forces Retirement Home,,,, ,,security@mail.gov,,\n" + "ddomain3.gov,On hold,(blank),2023-11-15,Federal," + "Armed Forces Retirement Home,,,, ,,security@mail.gov,,\n" "sdomain8.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" "xdomain7.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" "zdomain9.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n" @@ -786,10 +785,13 @@ def test_domain_request_data_full(self): "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,,,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,Tester,testy@town.com," - "Chief Tester,Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com," + '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," + "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,,,2,,,,0,1,city1.gov,,,,," "Purpose of the site,CISA-first-name CISA-last-name | There is more,Testy Tester testy2@town.com," diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index ed314812e..2758375b1 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -231,7 +231,7 @@ def export_data_to_csv(cls, csv_file, **kwargs): # Return rows that for easier parsing and testing return rows - + @classmethod def get_annotated_queryset(cls, **kwargs): """Returns an annotated queryset based off of all query conditions.""" @@ -546,7 +546,8 @@ def get_computed_fields(cls, **kwargs): # When portfolio is present, use its value instead When( Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), - then=F("portfolio__federal_agency__agency")), + then=F("portfolio__federal_agency__agency"), + ), # Otherwise, return the natively assigned value default=F("federal_agency__agency"), output_field=CharField(), @@ -769,20 +770,16 @@ def get_filtered_domain_infos_by_org(domain_infos_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value.""" annotated_queryset = domain_infos_to_filter.annotate( - converted_generic_org_type=Case( - # Recreate the logic of the converted_generic_org_type property - # here in annotations - When( - portfolio__isnull=False, - then=F("portfolio__organization_type") - ), - default=F("generic_org_type"), - output_field=CharField(), - ) + converted_generic_org_type=Case( + # Recreate the logic of the converted_generic_org_type property + # here in annotations + When(portfolio__isnull=False, then=F("portfolio__organization_type")), + default=F("generic_org_type"), + output_field=CharField(), ) + ) return annotated_queryset.filter(converted_generic_org_type=org_to_filter_by) - @classmethod def get_sliced_domains(cls, filter_condition): """Get filtered domains counts sliced by org type and election office. @@ -1614,19 +1611,16 @@ def model(cls): def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter_by): """Returns a list of Domain Requests that has been filtered by the given organization value""" annotated_queryset = domain_requests_to_filter.annotate( - converted_generic_org_type=Case( - # Recreate the logic of the converted_generic_org_type property - # here in annotations - When( - portfolio__isnull=False, - then=F("portfolio__organization_type") - ), - default=F("generic_org_type"), - output_field=CharField(), - ) + converted_generic_org_type=Case( + # Recreate the logic of the converted_generic_org_type property + # here in annotations + When(portfolio__isnull=False, then=F("portfolio__organization_type")), + default=F("generic_org_type"), + output_field=CharField(), ) + ) return annotated_queryset.filter(converted_generic_org_type=org_to_filter_by) - + # return domain_requests_to_filter.filter( # # Filter based on the generic org value returned by converted_generic_org_type # id__in=[ @@ -1636,9 +1630,6 @@ def get_filtered_domain_requests_by_org(domain_requests_to_filter, org_to_filter # and domainRequest.converted_generic_org_type == org_to_filter_by # ] # ) - - - @classmethod def get_computed_fields(cls, delimiter=", ", **kwargs): @@ -1661,21 +1652,10 @@ def get_computed_fields(cls, delimiter=", ", **kwargs): # When portfolio is present, use its value instead When( Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), - then=F("portfolio__federal_agency__agency") - ), - # Otherwise, return the natively assigned value - default=F("federal_agency__agency"), - output_field=CharField(), - ), - "converted_federal_type": Case( - # When portfolio is present, use its value instead - # NOTE: this is an @Property funciton in portfolio. - When( - Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False), - then=F("portfolio__federal_agency__federal_type"), + then=F("portfolio__federal_agency__agency"), ), # Otherwise, return the natively assigned value - default=F("federal_type"), + default=F("federal_agency__agency"), output_field=CharField(), ), "converted_federal_type": Case( diff --git a/src/registrar/views/portfolios.py b/src/registrar/views/portfolios.py index 429934dcf..880472509 100644 --- a/src/registrar/views/portfolios.py +++ b/src/registrar/views/portfolios.py @@ -625,4 +625,4 @@ def submit_new_member(self, form): else: if permission_exists: messages.warning(self.request, "User is already a member of this portfolio.") - return redirect(self.get_success_url()) \ No newline at end of file + return redirect(self.get_success_url()) From 0a36b10fb77816711a3cacd13c83add741ec566c Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 5 Dec 2024 10:43:14 -0700 Subject: [PATCH 27/29] more unit test adjustments --- src/registrar/tests/test_admin_domain.py | 2 +- src/registrar/tests/test_admin_request.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py index 26f675150..0a2af50db 100644 --- a/src/registrar/tests/test_admin_domain.py +++ b/src/registrar/tests/test_admin_domain.py @@ -730,7 +730,7 @@ def test_short_org_name_in_domains_list(self): # for our actual domain_request self.assertContains(response, "Federal", count=57) # This may be a bit more robust - self.assertContains(response, 'Federal', count=1) + self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist self.assertNotContains(response, "Federal: an agency of the U.S. government") diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index a903188f3..2440666f0 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -576,7 +576,7 @@ def test_short_org_name_in_domain_requests_list(self): response = self.client.get("/admin/registrar/domainrequest/?generic_org_type__exact=federal") # There are 2 template references to Federal (4) and two in the results data # of the request - self.assertContains(response, "Federal", count=51) + self.assertContains(response, "Federal", count=55) # This may be a bit more robust self.assertContains(response, 'federal', count=1) # Now let's make sure the long description does not exist From 77d4f0aee9531b3f8041e00b85b8ab2b0db4fe1e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 5 Dec 2024 12:59:13 -0700 Subject: [PATCH 28/29] unit test fix --- src/registrar/tests/test_admin_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 2440666f0..025c80363 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -578,7 +578,7 @@ def test_short_org_name_in_domain_requests_list(self): # of the request self.assertContains(response, "Federal", count=55) # This may be a bit more robust - self.assertContains(response, 'federal', count=1) + self.assertContains(response, 'Federal', count=1) # Now let's make sure the long description does not exist self.assertNotContains(response, "Federal: an agency of the U.S. government") From 4e1797a28055d4d540aa9f95aaaf7e077fd51bc1 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 5 Dec 2024 13:03:55 -0700 Subject: [PATCH 29/29] remove maxDiff = None --- src/registrar/tests/test_admin_request.py | 2 -- src/registrar/tests/test_migrations.py | 1 - src/registrar/tests/test_reports.py | 9 --------- 3 files changed, 12 deletions(-) diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 025c80363..d2e4c1c1b 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -1693,7 +1693,6 @@ def test_readonly_when_restricted_creator(self): "notes", "alternative_domains", ] - self.maxDiff = None self.assertEqual(readonly_fields, expected_fields) def test_readonly_fields_for_analyst(self): @@ -1702,7 +1701,6 @@ def test_readonly_fields_for_analyst(self): request.user = self.staffuser readonly_fields = self.admin.get_readonly_fields(request) - self.maxDiff = None expected_fields = [ "portfolio_senior_official", "portfolio_organization_type", diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index eaaae8727..11daca870 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -63,7 +63,6 @@ def test_groups_created(self): # Get the codenames of actual permissions associated with the group actual_permissions = [p.codename for p in cisa_analysts_group.permissions.all()] - self.maxDiff = None # Assert that the actual permissions match the expected permissions self.assertListEqual(actual_permissions, expected_permissions) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 4a868a794..d801ce76a 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -280,7 +280,6 @@ def test_domain_data_type(self): # 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) @less_console_noise_decorator @@ -327,7 +326,6 @@ def test_domain_data_type_user(self): # 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) @less_console_noise_decorator @@ -506,7 +504,6 @@ def test_domain_data_full(self): # 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) @less_console_noise_decorator @@ -546,14 +543,12 @@ def test_domain_data_federal(self): # 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) @less_console_noise_decorator def test_domain_growth(self): """Shows ready and deleted domains within a date range, sorted""" # Remove "Created at" and "First ready" because we can't guess this immutable, dynamically generated test data - self.maxDiff = None columns = [ "Domain name", "Domain type", @@ -616,7 +611,6 @@ def test_domain_managed(self): squeaker@rocks.com is invited to domain2 (DNS_NEEDED) and domain10 (No managers). She should show twice in this report but not in test_DomainManaged.""" - self.maxDiff = None # Create a CSV file in memory csv_file = StringIO() # Call the export functions @@ -651,7 +645,6 @@ def test_domain_managed(self): # 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) @less_console_noise_decorator @@ -729,7 +722,6 @@ def test_domain_request_growth(self): # @less_console_noise_decorator def test_domain_request_data_full(self): - self.maxDiff = None """Tests the full domain request report.""" # Remove "Submitted at" because we can't guess this immutable, dynamically generated test data columns = [ @@ -865,7 +857,6 @@ def test_member_export(self): # Create a request and add the user to the request request = self.factory.get("/") request.user = self.user - self.maxDiff = None # Add portfolio to session request = GenericTestHelper._mock_user_request_for_factory(request) request.session["portfolio"] = self.portfolio_1