From 9f9147e66181795ded63c97f0523199800dff2bb Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 1 Nov 2024 17:37:44 -0600 Subject: [PATCH 01/78] 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): # 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 @@ class Domain(TimeStampedModel, DomainHelper): 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 @@ class DomainInformation(TimeStampedModel): 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 @@ class DomainInformation(TimeStampedModel): if self.portfolio: return self.portfolio.city return self.city - + @property def converted_state_territory(self): if self.portfolio: @@ -492,3 +493,9 @@ class DomainInformation(TimeStampedModel): 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/78] 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 DomainInformationResource(resources.ModelResource): 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): orderable_fk_fields = [("domain", "name")] # Filters - list_filter = ["generic_org_type"] + list_filter = [GenericOrgFilter] # Search search_fields = [ @@ -1550,7 +1579,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): # 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/78] 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): ] # 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/78] 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ), ) + # ------- 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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/78] 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): "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/78] 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 | 108 ++++++++++----- src/registrar/views/report_views.py | 9 +- 6 files changed, 236 insertions(+), 55 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.utils.html import escape 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ urlpatterns = [ 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 @@ class DomainInformation(TimeStampedModel): @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 @@ class DomainInformation(TimeStampedModel): 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 @@ class DomainRequest(TimeStampedModel): @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 @@ class DomainRequest(TimeStampedModel): 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.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__) @@ -197,6 +202,8 @@ class BaseExport(ABC): 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 @@ class BaseExport(ABC): ) 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 @@ class DomainExport(BaseExport): 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 @@ class DomainExport(BaseExport): ): 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 @@ class DomainExport(BaseExport): "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 @@ class DomainExport(BaseExport): } 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 @@ class DomainExport(BaseExport): 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 @@ class DomainDataType(DomainExport): """ 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 @@ class DomainRequestsDataType: 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 @@ class DomainRequestExport(BaseExport): 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 @@ class DomainRequestExport(BaseExport): "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 @@ from registrar.utility import csv_export 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 @@ class ExportDataType(View): 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/78] 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.templatetags.custom_filters import get_region 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 @@ class BaseExport(ABC): 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 @@ class BaseExport(ABC): ) 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 @@ class DomainExport(BaseExport): } 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 014c38791092e95e008798c64f4a8898e67fb5c4 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:05:51 -0800 Subject: [PATCH 08/78] Add aria label to purpose form --- src/registrar/forms/domain_request_wizard.py | 4 +++- src/registrar/templates/domain_request_purpose.html | 2 +- src/registrar/templates/includes/input_with_errors.html | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index bfbc22124..5e156cc73 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -524,7 +524,9 @@ class DotGovDomainForm(RegistrarForm): class PurposeForm(RegistrarForm): purpose = forms.CharField( label="Purpose", - widget=forms.Textarea(), + widget=forms.Textarea(attrs={ + 'aria-label': 'What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used for a website, email, or something else? You can enter up to 2000 characters.' + }), validators=[ MaxLengthValidator( 2000, diff --git a/src/registrar/templates/domain_request_purpose.html b/src/registrar/templates/domain_request_purpose.html index bfd9beb15..8c6d417cd 100644 --- a/src/registrar/templates/domain_request_purpose.html +++ b/src/registrar/templates/domain_request_purpose.html @@ -13,7 +13,7 @@ {% endblock %} {% block form_fields %} - {% with attr_maxlength=2000 add_label_class="usa-sr-only" %} + {% with add_aria_label="test" attr_maxlength=2000 add_label_class="usa-sr-only" %} {% input_with_errors forms.0.purpose %} {% endwith %} {% endblock %} diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index d1e53968e..b3f5e520e 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -69,6 +69,8 @@ error messages, if necessary. {# this is the input field, itself #} {% include widget.template_name %} + + {% if append_gov %} .gov From bb2215bd75c91b773b6a69b3b3e309ce4f850790 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:11:28 -0800 Subject: [PATCH 09/78] Revert unwanted changes --- src/registrar/templates/domain_request_purpose.html | 2 +- src/registrar/templates/includes/input_with_errors.html | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/templates/domain_request_purpose.html b/src/registrar/templates/domain_request_purpose.html index 8c6d417cd..bfd9beb15 100644 --- a/src/registrar/templates/domain_request_purpose.html +++ b/src/registrar/templates/domain_request_purpose.html @@ -13,7 +13,7 @@ {% endblock %} {% block form_fields %} - {% with add_aria_label="test" attr_maxlength=2000 add_label_class="usa-sr-only" %} + {% with attr_maxlength=2000 add_label_class="usa-sr-only" %} {% input_with_errors forms.0.purpose %} {% endwith %} {% endblock %} diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index b3f5e520e..d1e53968e 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -69,8 +69,6 @@ error messages, if necessary. {# this is the input field, itself #} {% include widget.template_name %} - - {% if append_gov %} .gov From 1b034ba04b8a67598dc994822c7f82e7852552c3 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:27:43 -0800 Subject: [PATCH 10/78] Run linter --- src/registrar/forms/domain_request_wizard.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 5e156cc73..28044b0ec 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -524,9 +524,11 @@ class DotGovDomainForm(RegistrarForm): class PurposeForm(RegistrarForm): purpose = forms.CharField( label="Purpose", - widget=forms.Textarea(attrs={ - 'aria-label': 'What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used for a website, email, or something else? You can enter up to 2000 characters.' - }), + widget=forms.Textarea( + attrs={ + "aria-label": "What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used for a website, email, or something else? You can enter up to 2000 characters." + } + ), validators=[ MaxLengthValidator( 2000, From f79b4e58c7e3be12957c28bf5cee392939b03f7c Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:34:27 -0800 Subject: [PATCH 11/78] Split line length of long strong --- src/registrar/forms/domain_request_wizard.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 28044b0ec..080694f28 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -526,7 +526,8 @@ class PurposeForm(RegistrarForm): label="Purpose", widget=forms.Textarea( attrs={ - "aria-label": "What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used for a website, email, or something else? You can enter up to 2000 characters." + "aria-label": "What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used \ + for a website, email, or \something else? You can enter up to 2000 characters." } ), validators=[ From d6789ba2caa76a612dc3cc339bd3f72d37e061e9 Mon Sep 17 00:00:00 2001 From: Erin Song <121973038+erinysong@users.noreply.github.com> Date: Thu, 21 Nov 2024 11:39:13 -0800 Subject: [PATCH 12/78] Fix linter errors --- src/registrar/forms/domain_request_wizard.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 080694f28..c7f5571af 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -526,8 +526,8 @@ class PurposeForm(RegistrarForm): label="Purpose", widget=forms.Textarea( attrs={ - "aria-label": "What is the purpose of your requested domain? Describe how you’ll use your .gov domain. Will it be used \ - for a website, email, or \something else? You can enter up to 2000 characters." + "aria-label": "What is the purpose of your requested domain? Describe how you’ll use your .gov domain. \ + Will it be used for a website, email, or something else? You can enter up to 2000 characters." } ), validators=[ From 503d214ca256fc1c555daa51ed8daa3044747ec5 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Sun, 24 Nov 2024 22:01:02 -0700 Subject: [PATCH 13/78] 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 DomainRequestResource(FsmModelResource): # 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 DomainResource(FsmModelResource): ) # 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): # ------- 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 @@ class DomainInformation(TimeStampedModel): @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 14/78] 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 @@ class DomainRequest(TimeStampedModel): @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 @@ class DomainRequest(TimeStampedModel): 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 @@ class DomainRequest(TimeStampedModel): 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 15/78] 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 @@ from registrar.models import ( 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 @@ class DomainExport(BaseExport): "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 @@ class DomainExport(BaseExport): } row = [FIELDS.get(column, "") for column in columns] - return row @@ -537,7 +623,7 @@ class DomainDataType(DomainExport): """ 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 @@ class DomainDataType(DomainExport): """ 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 @@ class DomainRequestsDataType: 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 @@ class DomainDataFull(DomainExport): ], ) - @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 @@ class DomainDataFederal(DomainExport): ], ) - @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 @@ class DomainRequestExport(BaseExport): ] ) + + @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 @@ class DomainRequestExport(BaseExport): "Other contacts": model.get("all_other_contacts"), "Current websites": model.get("all_current_websites"), # Untouched FK fields - passed into the request dict. - "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 @@ class DomainRequestGrowth(DomainRequestExport): Get a list of fields from related tables. """ return ["requested_domain__name"] - - + class DomainRequestDataFull(DomainRequestExport): """ Shows all but STARTED requests @@ -1492,7 +1621,11 @@ class DomainRequestDataFull(DomainRequestExport): """ 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 @@ class DomainRequestDataFull(DomainRequestExport): 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 16/78] 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 17/78] 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 | 245 ++++++++++++++------- src/registrar/views/report_views.py | 9 +- 7 files changed, 263 insertions(+), 189 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 DomainInformationResource(resources.ModelResource): # 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 DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 DomainRequestResource(FsmModelResource): # 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 DomainResource(FsmModelResource): ) # 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): # 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): @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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): 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 @@ class Domain(TimeStampedModel, DomainHelper): 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 @@ class DomainInformation(TimeStampedModel): if self.portfolio: return self.portfolio.city return self.city - + @property def converted_state_territory(self): if self.portfolio: @@ -493,5 +493,3 @@ class DomainInformation(TimeStampedModel): 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 @@ class DomainRequest(TimeStampedModel): if self.portfolio: return self.portfolio.state_territory return self.state_territory - + @property def converted_urbanization(self): if self.portfolio: @@ -1477,4 +1477,4 @@ class DomainRequest(TimeStampedModel): 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ from registrar.models import ( 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 @@ class DomainExport(BaseExport): ), "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 @@ class DomainExport(BaseExport): ): 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 @@ class DomainExport(BaseExport): 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 @@ class DomainExport(BaseExport): 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 = ( - cls.get_filtered_domain_infos_by_org(domain_informations, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().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() + ) + 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 @@ class DomainDataType(DomainExport): """ 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 @@ class DomainRequestExport(BaseExport): ), "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 @@ class DomainRequestExport(BaseExport): 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() - state_or_territory = ( - cls.get_filtered_domain_requests_by_org(requests, DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).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() + ) + 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 @@ class DomainRequestGrowth(DomainRequestExport): Get a list of fields from related tables. """ return ["requested_domain__name"] - + + class DomainRequestDataFull(DomainRequestExport): """ Shows all but STARTED requests @@ -1625,24 +1696,28 @@ class DomainRequestDataFull(DomainRequestExport): 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 @@ from registrar.utility import csv_export 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 @@ class ExportDataType(View): 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 725441da70e4528a5b446fabe53de9beb244e2c1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 25 Nov 2024 14:59:53 -0700 Subject: [PATCH 18/78] Allow create portfolio script to run for multiple --- .../commands/create_federal_portfolio.py | 139 ++++++++---------- .../commands/utility/terminal_helper.py | 7 +- 2 files changed, 66 insertions(+), 80 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d05a2911b..e5f4be61c 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -13,16 +13,28 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.created_portfolios = [] + self.skipped_portfolios = [] + self.failed_portfolios = [] + def add_arguments(self, parser): """Add three arguments: 1. agency_name => the value of FederalAgency.agency 2. --parse_requests => if true, adds the given portfolio to each related DomainRequest 3. --parse_domains => if true, adds the given portfolio to each related DomainInformation """ - parser.add_argument( - "agency_name", + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument( + "--agency_name", help="The name of the FederalAgency to add", ) + group.add_argument( + "--branch", + choices=["executive", "legislative", "judicial"], + help="The federal branch to process. Creates a portfolio for each FederalAgency in this branch.", + ) parser.add_argument( "--parse_requests", action=argparse.BooleanOptionalAction, @@ -39,7 +51,9 @@ class Command(BaseCommand): help="Adds portfolio to both requests and domains", ) - def handle(self, agency_name, **options): + def handle(self, **options): + agency_name = options.get("agency_name") + branch = options.get("branch") parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") @@ -51,25 +65,40 @@ class Command(BaseCommand): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") - federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first() - if not federal_agency: - raise ValueError( - f"Cannot find the federal agency '{agency_name}' in our database. " - "The value you enter for `agency_name` must be " - "prepopulated in the FederalAgency table before proceeding." - ) + federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} + agencies = FederalAgency.objects.filter(**federal_agency_filter) + if not agencies or agencies.count() < 1: + if agency_name: + raise ValueError( + f"Cannot find the federal agency '{agency_name}' in our database. " + "The value you enter for `agency_name` must be " + "prepopulated in the FederalAgency table before proceeding." + ) + else: + raise ValueError(f"Cannot find '{branch}' federal agencies in our database.") - portfolio = self.create_or_modify_portfolio(federal_agency) - self.create_suborganizations(portfolio, federal_agency) + for federal_agency in agencies: + message = f"Processing federal agency '{federal_agency.agency}'..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + try: + portfolio, created = self.create_portfolio(federal_agency) + if created: + self.create_suborganizations(portfolio, federal_agency) + if parse_domains or both: + self.handle_portfolio_domains(portfolio, federal_agency) - if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) + if parse_requests or both: + self.handle_portfolio_requests(portfolio, federal_agency) - if parse_domains or both: - self.handle_portfolio_domains(portfolio, federal_agency) + except Exception as exec: + self.failed_portfolios.append(federal_agency) + logger.error(exec) + TerminalHelper.log_script_run_summary( + self.created_portfolios, self.failed_portfolios, self.skipped_portfolios, debug=False, + skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----" + ) - def create_or_modify_portfolio(self, federal_agency): - """Creates or modifies a portfolio record based on a federal agency.""" + def create_portfolio(self, federal_agency): portfolio_args = { "federal_agency": federal_agency, "organization_name": federal_agency.agency, @@ -84,8 +113,8 @@ class Command(BaseCommand): portfolio, created = Portfolio.objects.get_or_create( organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args ) - if created: + self.created_portfolios.append(portfolio) message = f"Created portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) @@ -99,36 +128,12 @@ class Command(BaseCommand): ) TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) else: - proceed = TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f""" - The given portfolio '{federal_agency.agency}' already exists in our DB. - If you cancel, the rest of the script will still execute but this record will not update. - """, - prompt_title="Do you wish to modify this record?", + self.skipped_portfolios.append(portfolio) + message = ( + f"The given portfolio '{portfolio}' already exists in our DB. Skipping create." ) - if proceed: - - # Don't override the creator and notes fields - if portfolio.creator: - portfolio_args.pop("creator") - - if portfolio.notes: - portfolio_args.pop("notes") - - # Update everything else - for key, value in portfolio_args.items(): - setattr(portfolio, key, value) - - portfolio.save() - message = f"Modified portfolio '{portfolio}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - - if portfolio_args.get("senior_official"): - message = f"Added/modified senior official '{portfolio_args['senior_official']}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - - return portfolio + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + return portfolio, created def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" @@ -149,7 +154,8 @@ class Command(BaseCommand): # Check if we need to update any existing suborgs first. This step is optional. existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): - self._update_existing_suborganizations(portfolio, existing_suborgs) + message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create." + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] @@ -175,29 +181,6 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def _update_existing_suborganizations(self, portfolio, orgs_to_update): - """ - Update existing suborganizations with new portfolio. - Prompts for user confirmation before proceeding. - """ - proceed = TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f"""Some suborganizations already exist in our DB. - If you cancel, the rest of the script will still execute but these records will not update. - - ==Proposed Changes== - The following suborgs will be updated: {[org.name for org in orgs_to_update]} - """, - prompt_title="Do you wish to modify existing suborganizations?", - ) - if proceed: - for org in orgs_to_update: - org.portfolio = portfolio - - Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) - message = f"Updated {len(orgs_to_update)} suborganizations." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. @@ -208,11 +191,11 @@ class Command(BaseCommand): DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(status__in=invalid_states) if not domain_requests.exists(): message = f""" - Portfolios not added to domain requests: no valid records found. - This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Portfolio '{portfolio}' not added to domain requests: no valid records found. + This means that a filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results. Excluded statuses: STARTED, INELIGIBLE, REJECTED. """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) @@ -234,11 +217,11 @@ class Command(BaseCommand): Associate portfolio with domains for a federal agency. Updates all relevant domain information records. """ - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) if not domain_infos.exists(): message = f""" - Portfolios not added to domains: no valid records found. - This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Portfolio '{portfolio}' not added to domains: no valid records found. + The filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results. """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index fa7cde683..22b5a7cc3 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -192,7 +192,7 @@ class PopulateScriptTemplate(ABC): class TerminalHelper: @staticmethod def log_script_run_summary( - to_update, failed_to_update, skipped, debug: bool, log_header=None, display_as_str=False + to_update, failed_to_update, skipped, debug: bool, log_header=None, skipped_header=None, display_as_str=False ): """Prints success, failed, and skipped counts, as well as all affected objects.""" @@ -203,6 +203,9 @@ class TerminalHelper: if log_header is None: log_header = "============= FINISHED ===============" + if skipped_header is None: + skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----" + # Prepare debug messages if debug: updated_display = [str(u) for u in to_update] if display_as_str else to_update @@ -236,7 +239,7 @@ class TerminalHelper: f"""{TerminalColors.YELLOW} {log_header} Updated {update_success_count} entries - ----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) ----- + {skipped_header} Skipped updating {update_skipped_count} entries {TerminalColors.ENDC} """ From e75c027738cc7c21a47404ea5931ef8a515baa2a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 25 Nov 2024 15:16:04 -0700 Subject: [PATCH 19/78] 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class DomainExport(BaseExport): """ 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 @@ class DomainExport(BaseExport): 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 @@ class DomainDataType(DomainExport): "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 @@ class DomainDataType(DomainExport): """ # 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 @@ class DomainRequestsDataType: 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 @@ class DomainDataFull(DomainExport): "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 @@ class DomainDataFull(DomainExport): """ # 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 @@ class DomainDataFederal(DomainExport): "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 @@ class DomainDataFederal(DomainExport): """ # 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 @@ class DomainRequestExport(BaseExport): """ 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( @@ -1488,11 +1541,11 @@ class DomainRequestExport(BaseExport): """ # 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 20/78] 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class CsvReportsTest(MockDbForSharedTests): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 21/78] 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ) 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ): 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 22/78] 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 @@ class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin): ): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): "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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class DomainExport(BaseExport): "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 @@ class DomainDataType(DomainExport): "Domain managers", "Invited domain managers", ] - @classmethod def get_annotations_for_sort(cls, delimiter=", "): @@ -852,7 +854,7 @@ class DomainDataFull(DomainExport): "State", "Security contact email", ] - + @classmethod def get_annotations_for_sort(cls, delimiter=", "): """ @@ -940,14 +942,13 @@ class DomainDataFederal(DomainExport): "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 @@ class DomainRequestExport(BaseExport): "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 7c9b15efd272cba0306592fe07d08615089cc9d0 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 26 Nov 2024 11:32:45 -0500 Subject: [PATCH 23/78] Fix duplicate portfolio permission error --- src/registrar/tests/test_admin.py | 57 ++++++++++++++++++++++++++++ src/registrar/views/transfer_user.py | 4 ++ 2 files changed, 61 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2677462df..67635a0bb 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -63,6 +63,7 @@ from .common import ( from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model +from django.contrib import messages from unittest.mock import ANY, patch, Mock @@ -2252,6 +2253,33 @@ class TestTransferUser(WebTest): self.assertEquals(user_portfolio_permission.user, self.user1) + @less_console_noise_decorator + def test_transfer_user_transfers_user_portfolio_roles_no_error_when_duplicates(self): + """Assert that duplicate portfolio user roles do not throw errorsd""" + portfolio1 = Portfolio.objects.create(organization_name="Hotel California", creator=self.user2) + UserPortfolioPermission.objects.create( + user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + UserPortfolioPermission.objects.create( + user=self.user2, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + + with patch.object(messages, "error"): + user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk])) + + submit_form = user_transfer_page.forms[1] + submit_form["selected_user"] = self.user2.pk + submit_form.submit() + + # Verify portfolio permissions remain valid for the original user + self.assertTrue( + UserPortfolioPermission.objects.filter( + user=self.user1, portfolio=portfolio1, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ).exists() + ) + + messages.error.assert_not_called() + @less_console_noise_decorator def test_transfer_user_transfers_domain_request_creator_and_investigator(self): """Assert that domain request fields get transferred""" @@ -2306,6 +2334,35 @@ class TestTransferUser(WebTest): self.assertEquals(user_domain_role1.user, self.user1) self.assertEquals(user_domain_role2.user, self.user1) + @less_console_noise_decorator + def test_transfer_user_transfers_domain_role_no_error_when_duplicate(self): + """Assert that duplicate user domain roles do not throw errors""" + domain_1, _ = Domain.objects.get_or_create(name="chrome.gov", state=Domain.State.READY) + domain_2, _ = Domain.objects.get_or_create(name="v8.gov", state=Domain.State.READY) + UserDomainRole.objects.get_or_create(user=self.user1, domain=domain_1, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.get_or_create(user=self.user2, domain=domain_1, role=UserDomainRole.Roles.MANAGER) + UserDomainRole.objects.get_or_create(user=self.user2, domain=domain_2, role=UserDomainRole.Roles.MANAGER) + + with patch.object(messages, "error"): + + user_transfer_page = self.app.get(reverse("transfer_user", args=[self.user1.pk])) + submit_form = user_transfer_page.forms[1] + submit_form["selected_user"] = self.user2.pk + submit_form.submit() + + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user1, domain=domain_1, role=UserDomainRole.Roles.MANAGER + ).exists() + ) + self.assertTrue( + UserDomainRole.objects.filter( + user=self.user1, domain=domain_2, role=UserDomainRole.Roles.MANAGER + ).exists() + ) + + messages.error.assert_not_called() + @less_console_noise_decorator def test_transfer_user_transfers_verified_by_staff_requestor(self): """Assert that verified by staff creator gets transferred""" diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py index ac51cd20b..69a0f4997 100644 --- a/src/registrar/views/transfer_user.py +++ b/src/registrar/views/transfer_user.py @@ -116,6 +116,10 @@ class TransferUserView(View): if model_class.objects.filter(user=current_user, domain=obj.domain).exists(): continue # Skip the update to avoid a duplicate + if model_class == UserPortfolioPermission: + if model_class.objects.filter(user=current_user, portfolio=obj.portfolio).exists(): + continue # Skip the update to avoid a duplicate + # Update the field on the object and save it setattr(obj, field_name, current_user) obj.save() From 7c1e232d6b4fd6a39313eda1136a123851af52db Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 10:14:44 -0700 Subject: [PATCH 24/78] cleanup --- .../commands/create_federal_portfolio.py | 85 +++++++++++-------- .../commands/utility/terminal_helper.py | 23 ++++- 2 files changed, 69 insertions(+), 39 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index e5f4be61c..4c0856fb4 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -15,9 +15,9 @@ class Command(BaseCommand): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.created_portfolios = [] - self.skipped_portfolios = [] - self.failed_portfolios = [] + self.updated_portfolios = set() + self.skipped_portfolios = set() + self.failed_portfolios = set() def add_arguments(self, parser): """Add three arguments: @@ -91,49 +91,61 @@ class Command(BaseCommand): self.handle_portfolio_requests(portfolio, federal_agency) except Exception as exec: - self.failed_portfolios.append(federal_agency) + self.failed_portfolios.add(federal_agency) logger.error(exec) + message = f"Failed to create portfolio '{portfolio}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message) + TerminalHelper.log_script_run_summary( - self.created_portfolios, self.failed_portfolios, self.skipped_portfolios, debug=False, - skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----" + self.updated_portfolios, + self.failed_portfolios, + self.skipped_portfolios, + debug=False, + skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----", + display_as_str=True, ) def create_portfolio(self, federal_agency): - portfolio_args = { - "federal_agency": federal_agency, - "organization_name": federal_agency.agency, - "organization_type": DomainRequest.OrganizationChoices.FEDERAL, - "creator": User.get_default_user(), - "notes": "Auto-generated record", - } + # Get the org name / senior official + org_name = federal_agency.agency + so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None - if federal_agency.so_federal_agency.exists(): - portfolio_args["senior_official"] = federal_agency.so_federal_agency.first() + # First just try to get an existing portfolio + portfolio = Portfolio.objects.filter(organization_name=org_name).first() + if portfolio: + self.skipped_portfolios.add(portfolio) + TerminalHelper.colorful_logger( + logger.info, + TerminalColors.YELLOW, + f"Portfolio with organization name '{org_name}' already exists. Skipping create.", + ) + return portfolio, False - portfolio, created = Portfolio.objects.get_or_create( - organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args + # Create new portfolio if it doesn't exist + portfolio = Portfolio.objects.create( + organization_name=org_name, + federal_agency=federal_agency, + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=User.get_default_user(), + notes="Auto-generated record", + senior_official=so, ) - if created: - self.created_portfolios.append(portfolio) - message = f"Created portfolio '{portfolio}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - if portfolio_args.get("senior_official"): - message = f"Added senior official '{portfolio_args['senior_official']}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - else: - message = ( - f"No senior official added to portfolio '{portfolio}'. " - "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + self.updated_portfolios.add(portfolio) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'") + + # Log if the senior official was added or not. + if portfolio.senior_official: + message = f"Added senior official '{portfolio.senior_official}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) else: - self.skipped_portfolios.append(portfolio) message = ( - f"The given portfolio '{portfolio}' already exists in our DB. Skipping create." + f"No senior official added to portfolio '{org_name}'. " + "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" ) TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - return portfolio, created + + return portfolio, True def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" @@ -155,7 +167,7 @@ class Command(BaseCommand): existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create." - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message) # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] @@ -191,7 +203,9 @@ class Command(BaseCommand): DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(status__in=invalid_states) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( + status__in=invalid_states + ) if not domain_requests.exists(): message = f""" Portfolio '{portfolio}' not added to domain requests: no valid records found. @@ -207,6 +221,7 @@ class Command(BaseCommand): domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 22b5a7cc3..0352fd3bc 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -206,8 +206,18 @@ class TerminalHelper: if skipped_header is None: skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----" + # Give the user the option to see failed / skipped records if any exist. + debug_anyway = False + if not debug and update_failed_count > 0 or update_skipped_count > 0: + debug_anyway = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"You will see {update_failed_count} failed and {update_skipped_count} skipped records.", + verify_message="** Some records were skipped, or some failed to update. **", + prompt_title="Do you wish to see the full list of failed, skipped and updated records?", + ) + # Prepare debug messages - if debug: + if debug or debug_anyway: updated_display = [str(u) for u in to_update] if display_as_str else to_update skipped_display = [str(s) for s in skipped] if display_as_str else skipped failed_display = [str(f) for f in failed_to_update] if display_as_str else failed_to_update @@ -220,7 +230,7 @@ class TerminalHelper: # Print out a list of everything that was changed, if we have any changes to log. # Otherwise, don't print anything. TerminalHelper.print_conditional( - debug, + True, f"{debug_messages.get('success') if update_success_count > 0 else ''}" f"{debug_messages.get('skipped') if update_skipped_count > 0 else ''}" f"{debug_messages.get('failed') if update_failed_count > 0 else ''}", @@ -371,7 +381,9 @@ class TerminalHelper: logger.info(print_statement) @staticmethod - def prompt_for_execution(system_exit_on_terminate: bool, prompt_message: str, prompt_title: str) -> bool: + def prompt_for_execution( + system_exit_on_terminate: bool, prompt_message: str, prompt_title: str, verify_message=None + ) -> bool: """Create to reduce code complexity. Prompts the user to inspect the given string and asks if they wish to proceed. @@ -383,6 +395,9 @@ class TerminalHelper: if system_exit_on_terminate: action_description_for_selecting_no = "exit" + if verify_message is None: + verify_message = "*** IMPORTANT: VERIFY THE FOLLOWING LOOKS CORRECT ***" + # Allow the user to inspect the command string # and ask if they wish to proceed proceed_execution = TerminalHelper.query_yes_no_exit( @@ -390,7 +405,7 @@ class TerminalHelper: ===================================================== {prompt_title} ===================================================== - *** IMPORTANT: VERIFY THE FOLLOWING LOOKS CORRECT *** + {verify_message} {prompt_message} {TerminalColors.FAIL} From 36c19d65bf4edef8c74707123f4867f5eeae4e8f Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 26 Nov 2024 13:31:01 -0700 Subject: [PATCH 25/78] 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 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin): }, ), (".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 @@ from unittest.mock import ANY, call, patch import boto3_mocking # type: ignore import logging -logger = logging.getLogger(__name__) - - class TestDomainAdminAsStaff(MockEppLib): """Test DomainAdmin class as staff user. @@ -494,7 +491,7 @@ class TestDomainAdminWithClient(TestCase): 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 @@ class TestDomainAdminWithClient(TestCase): 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 26/78] 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.db import models 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 @@ class UserPortfolioPermission(TimeStampedModel): 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 .common import ( 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 @@ class BaseExport(ABC): # 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 @@ class BaseExport(ABC): 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 @@ class PortfolioMembersJson(PortfolioMembersPermission, View): "svg_icon": ("visibility" if view_only else "settings"), } return member_json - From 7f8c0b9196f2e12b0cea34c9230247f94950e604 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:46:22 -0700 Subject: [PATCH 27/78] fix some tests --- src/registrar/tests/test_management_scripts.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 9fcd261f7..faa2313de 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1456,18 +1456,18 @@ class TestCreateFederalPortfolio(TestCase): User.objects.all().delete() @less_console_noise_decorator - def run_create_federal_portfolio(self, agency_name, parse_requests=False, parse_domains=False): + def run_create_federal_portfolio(self, **kwargs): with patch( "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", return_value=True, ): call_command( - "create_federal_portfolio", agency_name, parse_requests=parse_requests, parse_domains=parse_domains + "create_federal_portfolio", **kwargs ) def test_create_or_modify_portfolio(self): """Test portfolio creation and modification with suborg and senior official.""" - self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) self.assertEqual(portfolio.organization_name, self.federal_agency.agency) @@ -1485,7 +1485,7 @@ class TestCreateFederalPortfolio(TestCase): def test_handle_portfolio_requests(self): """Verify portfolio association with domain requests.""" - self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) self.domain_request.refresh_from_db() self.assertIsNotNone(self.domain_request.portfolio) @@ -1494,7 +1494,7 @@ class TestCreateFederalPortfolio(TestCase): def test_handle_portfolio_domains(self): """Check portfolio association with domain information.""" - self.run_create_federal_portfolio("Test Federal Agency", parse_domains=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_domains=True) self.domain_info.refresh_from_db() self.assertIsNotNone(self.domain_info.portfolio) @@ -1503,7 +1503,7 @@ class TestCreateFederalPortfolio(TestCase): def test_handle_parse_both(self): """Ensure correct parsing of both requests and domains.""" - self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True, parse_domains=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True, parse_domains=True) self.domain_request.refresh_from_db() self.domain_info.refresh_from_db() @@ -1516,7 +1516,7 @@ class TestCreateFederalPortfolio(TestCase): with self.assertRaisesRegex( CommandError, "You must specify at least one of --parse_requests or --parse_domains." ): - self.run_create_federal_portfolio("Test Federal Agency") + self.run_create_federal_portfolio(agency_name="Test Federal Agency") def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" @@ -1525,7 +1525,7 @@ class TestCreateFederalPortfolio(TestCase): "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding." ) with self.assertRaisesRegex(ValueError, expected_message): - self.run_create_federal_portfolio("Non-existent Agency", parse_requests=True) + self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True) def test_update_existing_portfolio(self): """Test updating an existing portfolio.""" @@ -1538,7 +1538,7 @@ class TestCreateFederalPortfolio(TestCase): notes="Old notes", ) - self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) existing_portfolio.refresh_from_db() self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) From aaa16842df9bab5c10dff3f6985511d8f61a29cf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Nov 2024 09:44:52 -0700 Subject: [PATCH 28/78] Doc update --- docs/operations/data_migration.md | 22 ++++++++++++------- .../commands/create_federal_portfolio.py | 11 +++++++--- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index a234d882b..0863aa0b7 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -893,22 +893,28 @@ Example: `cf ssh getgov-za` [Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. #### Step 5: Running the script -```./manage.py create_federal_portfolio "{federal_agency_name}" --both``` - +To create a specific portfolio: +```./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` Example (only requests): `./manage.py create_federal_portfolio "AMTRAK" --parse_requests` +To create a portfolios for all federal agencies in a branch: +```./manage.py create_federal_portfolio --branch "{executive|legislative|judicial}" --both``` +Example (only requests): `./manage.py create_federal_portfolio --branch "executive" --parse_requests` + ### Running locally #### Step 1: Running the script -```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --both``` +```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` ##### Parameters | | Parameter | Description | |:-:|:-------------------------- |:-------------------------------------------------------------------------------------------| -| 1 | **federal_agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | -| 2 | **both** | If True, runs parse_requests and parse_domains. | -| 3 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | -| 4 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | +| 1 | **agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | +| 2 | **branch** | Creates a portfolio for each federal agency in a branch: executive, legislative, judicial | +| 3 | **both** | If True, runs parse_requests and parse_domains. | +| 4 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | +| 5 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | -Note: Regarding parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, +- Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both. +- Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, you must specify at least one to run this script. diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 4c0856fb4..48202a93b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -14,6 +14,7 @@ class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" def __init__(self, *args, **kwargs): + """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" super().__init__(*args, **kwargs) self.updated_portfolios = set() self.skipped_portfolios = set() @@ -209,8 +210,11 @@ class Command(BaseCommand): if not domain_requests.exists(): message = f""" Portfolio '{portfolio}' not added to domain requests: no valid records found. - This means that a filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results. + This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. Excluded statuses: STARTED, INELIGIBLE, REJECTED. + Filter info: DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( + status__in=invalid_states + ) """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None @@ -236,7 +240,8 @@ class Command(BaseCommand): if not domain_infos.exists(): message = f""" Portfolio '{portfolio}' not added to domains: no valid records found. - The filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results. + The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Filter info: DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None @@ -249,5 +254,5 @@ class Command(BaseCommand): domain_info.sub_organization = suborgs.get(domain_info.organization_name) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) From 4beec038c7a39d4b9f3f7bb8b76883dcbe86f4dc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:25:15 -0700 Subject: [PATCH 29/78] Unit tests --- .../tests/test_management_scripts.py | 189 +++++++++++++++++- 1 file changed, 179 insertions(+), 10 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index faa2313de..47cf61d6a 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1421,10 +1421,27 @@ class TestCreateFederalPortfolio(TestCase): def setUp(self): self.mock_client = MockSESClient() self.user = User.objects.create(username="testuser") + + # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + + # And create some with federal_type ones with creative names + self.executive_agency_1 = FederalAgency.objects.create(agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE) + self.executive_agency_2 = FederalAgency.objects.create(agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE) + self.executive_agency_3 = FederalAgency.objects.create(agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE) + self.legislative_agency_1 = FederalAgency.objects.create(agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE) + self.legislative_agency_2 = FederalAgency.objects.create(agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE) + self.judicial_agency_1 = FederalAgency.objects.create(agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL) + self.judicial_agency_2 = FederalAgency.objects.create(agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL) self.senior_official = SeniorOfficial.objects.create( first_name="first", last_name="last", email="testuser@igorville.gov", federal_agency=self.federal_agency ) + self.executive_so_1 = SeniorOfficial.objects.create( + first_name="first", last_name="last", email="apple@igorville.gov", federal_agency=self.executive_agency_1 + ) + self.executive_so_2 = SeniorOfficial.objects.create( + first_name="first", last_name="last", email="mango@igorville.gov", federal_agency=self.executive_agency_2 + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): self.domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.IN_REVIEW, @@ -1436,7 +1453,7 @@ class TestCreateFederalPortfolio(TestCase): self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get() self.domain_request_2 = completed_domain_request( - name="sock@igorville.org", + name="icecreamforigorville.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, @@ -1446,6 +1463,28 @@ class TestCreateFederalPortfolio(TestCase): self.domain_request_2.approve() self.domain_info_2 = DomainInformation.objects.filter(domain_request=self.domain_request_2).get() + self.domain_request_3 = completed_domain_request( + name="exec_1.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.executive_agency_1, + user=self.user, + organization_name="Executive Agency 1", + ) + self.domain_request_3.approve() + self.domain_info_3 = self.domain_request_3.DomainRequest_info + + self.domain_request_4 = completed_domain_request( + name="exec_2.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.executive_agency_2, + user=self.user, + organization_name="Executive Agency 2", + ) + self.domain_request_4.approve() + self.domain_info_4 = self.domain_request_4.DomainRequest_info + def tearDown(self): DomainInformation.objects.all().delete() DomainRequest.objects.all().delete() @@ -1465,8 +1504,8 @@ class TestCreateFederalPortfolio(TestCase): "create_federal_portfolio", **kwargs ) - def test_create_or_modify_portfolio(self): - """Test portfolio creation and modification with suborg and senior official.""" + def test_create_single_portfolio(self): + """Test portfolio creation with suborg and senior official.""" self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1483,6 +1522,115 @@ class TestCreateFederalPortfolio(TestCase): # Test the senior official self.assertEqual(portfolio.senior_official, self.senior_official) + def test_create_multiple_portfolios_for_branch(self): + """Tests creating all portfolios under a given branch""" + federal_choice = DomainRequest.OrganizationChoices.FEDERAL + + # == Test creating executive portfolios == # + expected_portfolio_names = { + self.executive_agency_1.agency, + self.executive_agency_2.agency, + self.executive_agency_3.agency + } + self.run_create_federal_portfolio(branch="executive", parse_requests=True, parse_domains=True) + + # Ensure that all the portfolios we expect to get created were created + portfolios = Portfolio.objects.all() + self.assertEqual(portfolios.count(), 3) + + # Test that all created portfolios have the correct values + org_names, org_types, creators, notes, senior_officials = [], [], [], [], [] + for portfolio in portfolios: + org_names.append(portfolio.organization_name) + org_types.append(portfolio.organization_type) + creators.append(portfolio.creator) + notes.append(portfolio.notes) + senior_officials.append(portfolio.senior_official) + + # Test organization_name, organization_type, creator, and notes (in that order) + self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) + self.assertTrue(all([org_type == federal_choice for org_type in org_types])) + self.assertTrue(all([creator == User.get_default_user() for creator in creators])) + self.assertTrue(all([note == "Auto-generated record" for note in notes])) + + # Test senior officials were assigned correctly + expected_senior_officials = { + self.executive_so_1, + self.executive_so_2 + } + self.assertTrue(all([senior_official in expected_senior_officials for senior_official in senior_officials])) + + # Test that domain requests / domains were assigned correctly + expected_requests = DomainRequest.objects.filter(portfolio__id__in=[ + self.domain_request_3.id, + self.domain_request_4.id + ]) + expected_domain_infos = DomainInformation.objects.filter( + portfolio__id__in=[ + self.domain_info_3.id, + self.domain_info_4.id + ] + ) + self.assertEqual(expected_requests.count(), 2) + self.assertEqual(expected_domain_infos.count(), 2) + # == Test creating legislative portfolios == # + + # Clear old portfolios + Portfolio.objects.all().delete() + + expected_portfolio_names = { + self.legislative_agency_1.agency, + self.legislative_agency_2.agency, + } + self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True) + + # Ensure that all the portfolios we expect to get created were created + portfolios = Portfolio.objects.all() + self.assertEqual(portfolios.count(), 2) + + # Test that all created portfolios have the correct values + org_names, org_types, creators, notes = [], [], [], [] + for portfolio in portfolios: + org_names.append(portfolio.organization_name) + org_types.append(portfolio.organization_type) + creators.append(portfolio.creator) + notes.append(portfolio.notes) + + # Test organization_name, organization_type, creator, and notes (in that order) + self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) + self.assertTrue(all([org_type == federal_choice for org_type in org_types])) + self.assertTrue(all([creator == User.get_default_user() for creator in creators])) + self.assertTrue(all([note == "Auto-generated record" for note in notes])) + + # == Test creating judicial portfolios == # + + # Clear old portfolios + Portfolio.objects.all().delete() + + expected_portfolio_names = { + self.judicial_agency_1.agency, + self.judicial_agency_2.agency, + } + self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True) + + # Ensure that all the portfolios we expect to get created were created + portfolios = Portfolio.objects.all() + self.assertEqual(portfolios.count(), 2) + + # Test that all created portfolios have the correct values + org_names, org_types, creators, notes = [], [], [], [] + for portfolio in portfolios: + org_names.append(portfolio.organization_name) + org_types.append(portfolio.organization_type) + creators.append(portfolio.creator) + notes.append(portfolio.notes) + + # Test organization_name, organization_type, creator, and notes (in that order) + self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) + self.assertTrue(all([org_type == federal_choice for org_type in org_types])) + self.assertTrue(all([creator == User.get_default_user() for creator in creators])) + self.assertTrue(all([note == "Auto-generated record" for note in notes])) + def test_handle_portfolio_requests(self): """Verify portfolio association with domain requests.""" self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) @@ -1511,12 +1659,30 @@ class TestCreateFederalPortfolio(TestCase): self.assertIsNotNone(self.domain_info.portfolio) self.assertEqual(self.domain_request.portfolio, self.domain_info.portfolio) - def test_command_error_no_parse_options(self): - """Verify error when no parse options are provided.""" + def test_command_error_parse_options(self): + """Verify error when bad parse options are provided.""" + # The command should enforce either --branch or --agency_name + with self.assertRaisesRegex( + CommandError, "Error: one of the arguments --agency_name --branch is required" + ): + self.run_create_federal_portfolio() + + # We should forbid both at the same time + with self.assertRaisesRegex( + CommandError, "Error: argument --branch: not allowed with argument --agency_name" + ): + self.run_create_federal_portfolio(agency_name="test", branch="executive") + + # We expect a error to be thrown when we dont pass parse requests or domains with self.assertRaisesRegex( CommandError, "You must specify at least one of --parse_requests or --parse_domains." ): - self.run_create_federal_portfolio(agency_name="Test Federal Agency") + self.run_create_federal_portfolio(branch="executive") + + with self.assertRaisesRegex( + CommandError, "You must specify at least one of --parse_requests or --parse_domains." + ): + self.run_create_federal_portfolio(agency_name="test") def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" @@ -1527,8 +1693,8 @@ class TestCreateFederalPortfolio(TestCase): with self.assertRaisesRegex(ValueError, expected_message): self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True) - def test_update_existing_portfolio(self): - """Test updating an existing portfolio.""" + def test_does_not_update_existing_portfolio(self): + """Tests that an existing portfolio is not updated""" # Create an existing portfolio existing_portfolio = Portfolio.objects.create( federal_agency=self.federal_agency, @@ -1541,9 +1707,12 @@ class TestCreateFederalPortfolio(TestCase): self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) existing_portfolio.refresh_from_db() - self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) - self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL) + # SANITY CHECK: if the portfolio updates, it will change to FEDERAL. + # if this case fails, it means we are overriding data (and not simply just other weirdness) + self.assertNotEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL) # Notes and creator should be untouched + self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.CITY) + self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) From 022a63dc30e10366536e24b9d96bc9f515ad6a3d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Nov 2024 13:48:33 -0700 Subject: [PATCH 30/78] Cleanup --- .../commands/create_federal_portfolio.py | 6 +- .../tests/test_management_scripts.py | 185 ++++++++++-------- 2 files changed, 103 insertions(+), 88 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 48202a93b..9e2598595 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -70,13 +70,13 @@ class Command(BaseCommand): agencies = FederalAgency.objects.filter(**federal_agency_filter) if not agencies or agencies.count() < 1: if agency_name: - raise ValueError( + raise CommandError( f"Cannot find the federal agency '{agency_name}' in our database. " "The value you enter for `agency_name` must be " "prepopulated in the FederalAgency table before proceeding." ) else: - raise ValueError(f"Cannot find '{branch}' federal agencies in our database.") + raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." @@ -167,7 +167,7 @@ class Command(BaseCommand): # Check if we need to update any existing suborgs first. This step is optional. existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): - message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create." + message = f"Some suborganizations already exist for portfolio '{portfolio}'." TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message) # Create new suborgs, as long as they don't exist in the db already diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 47cf61d6a..429aed0d9 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1426,13 +1426,27 @@ class TestCreateFederalPortfolio(TestCase): self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") # And create some with federal_type ones with creative names - self.executive_agency_1 = FederalAgency.objects.create(agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE) - self.executive_agency_2 = FederalAgency.objects.create(agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE) - self.executive_agency_3 = FederalAgency.objects.create(agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE) - self.legislative_agency_1 = FederalAgency.objects.create(agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE) - self.legislative_agency_2 = FederalAgency.objects.create(agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE) - self.judicial_agency_1 = FederalAgency.objects.create(agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL) - self.judicial_agency_2 = FederalAgency.objects.create(agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL) + self.executive_agency_1 = FederalAgency.objects.create( + agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE + ) + self.executive_agency_2 = FederalAgency.objects.create( + agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE + ) + self.executive_agency_3 = FederalAgency.objects.create( + agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE + ) + self.legislative_agency_1 = FederalAgency.objects.create( + agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE + ) + self.legislative_agency_2 = FederalAgency.objects.create( + agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE + ) + self.judicial_agency_1 = FederalAgency.objects.create( + agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL + ) + self.judicial_agency_2 = FederalAgency.objects.create( + agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL + ) self.senior_official = SeniorOfficial.objects.create( first_name="first", last_name="last", email="testuser@igorville.gov", federal_agency=self.federal_agency ) @@ -1500,9 +1514,7 @@ class TestCreateFederalPortfolio(TestCase): "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", return_value=True, ): - call_command( - "create_federal_portfolio", **kwargs - ) + call_command("create_federal_portfolio", **kwargs) def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" @@ -1522,7 +1534,61 @@ class TestCreateFederalPortfolio(TestCase): # Test the senior official self.assertEqual(portfolio.senior_official, self.senior_official) - def test_create_multiple_portfolios_for_branch(self): + def test_create_multiple_portfolios_for_branch_judicial(self): + """Tests creating all portfolios under a given branch""" + federal_choice = DomainRequest.OrganizationChoices.FEDERAL + expected_portfolio_names = { + self.judicial_agency_1.agency, + self.judicial_agency_2.agency, + } + self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True) + + # Ensure that all the portfolios we expect to get created were created + portfolios = Portfolio.objects.all() + self.assertEqual(portfolios.count(), 2) + + # Test that all created portfolios have the correct values + org_names, org_types, creators, notes = [], [], [], [] + for portfolio in portfolios: + org_names.append(portfolio.organization_name) + org_types.append(portfolio.organization_type) + creators.append(portfolio.creator) + notes.append(portfolio.notes) + + # Test organization_name, organization_type, creator, and notes (in that order) + self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) + self.assertTrue(all([org_type == federal_choice for org_type in org_types])) + self.assertTrue(all([creator == User.get_default_user() for creator in creators])) + self.assertTrue(all([note == "Auto-generated record" for note in notes])) + + def test_create_multiple_portfolios_for_branch_legislative(self): + """Tests creating all portfolios under a given branch""" + federal_choice = DomainRequest.OrganizationChoices.FEDERAL + expected_portfolio_names = { + self.legislative_agency_1.agency, + self.legislative_agency_2.agency, + } + self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True) + + # Ensure that all the portfolios we expect to get created were created + portfolios = Portfolio.objects.all() + self.assertEqual(portfolios.count(), 2) + + # Test that all created portfolios have the correct values + org_names, org_types, creators, notes = [], [], [], [] + for portfolio in portfolios: + org_names.append(portfolio.organization_name) + org_types.append(portfolio.organization_type) + creators.append(portfolio.creator) + notes.append(portfolio.notes) + + # Test organization_name, organization_type, creator, and notes (in that order) + self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) + self.assertTrue(all([org_type == federal_choice for org_type in org_types])) + self.assertTrue(all([creator == User.get_default_user() for creator in creators])) + self.assertTrue(all([note == "Auto-generated record" for note in notes])) + + def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" federal_choice = DomainRequest.OrganizationChoices.FEDERAL @@ -1530,7 +1596,7 @@ class TestCreateFederalPortfolio(TestCase): expected_portfolio_names = { self.executive_agency_1.agency, self.executive_agency_2.agency, - self.executive_agency_3.agency + self.executive_agency_3.agency, } self.run_create_federal_portfolio(branch="executive", parse_requests=True, parse_domains=True) @@ -1556,80 +1622,33 @@ class TestCreateFederalPortfolio(TestCase): # Test senior officials were assigned correctly expected_senior_officials = { self.executive_so_1, - self.executive_so_2 + self.executive_so_2, + # We expect one record to skip + None, } self.assertTrue(all([senior_official in expected_senior_officials for senior_official in senior_officials])) # Test that domain requests / domains were assigned correctly - expected_requests = DomainRequest.objects.filter(portfolio__id__in=[ - self.domain_request_3.id, - self.domain_request_4.id - ]) + self.domain_request_3.refresh_from_db() + self.domain_request_4.refresh_from_db() + self.domain_info_3.refresh_from_db() + self.domain_info_4.refresh_from_db() + expected_requests = DomainRequest.objects.filter( + portfolio__id__in=[ + # Implicity tests for existence + self.domain_request_3.portfolio.id, + self.domain_request_4.portfolio.id, + ] + ) expected_domain_infos = DomainInformation.objects.filter( portfolio__id__in=[ - self.domain_info_3.id, - self.domain_info_4.id + # Implicity tests for existence + self.domain_info_3.portfolio.id, + self.domain_info_4.portfolio.id, ] ) self.assertEqual(expected_requests.count(), 2) self.assertEqual(expected_domain_infos.count(), 2) - # == Test creating legislative portfolios == # - - # Clear old portfolios - Portfolio.objects.all().delete() - - expected_portfolio_names = { - self.legislative_agency_1.agency, - self.legislative_agency_2.agency, - } - self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True) - - # Ensure that all the portfolios we expect to get created were created - portfolios = Portfolio.objects.all() - self.assertEqual(portfolios.count(), 2) - - # Test that all created portfolios have the correct values - org_names, org_types, creators, notes = [], [], [], [] - for portfolio in portfolios: - org_names.append(portfolio.organization_name) - org_types.append(portfolio.organization_type) - creators.append(portfolio.creator) - notes.append(portfolio.notes) - - # Test organization_name, organization_type, creator, and notes (in that order) - self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) - self.assertTrue(all([org_type == federal_choice for org_type in org_types])) - self.assertTrue(all([creator == User.get_default_user() for creator in creators])) - self.assertTrue(all([note == "Auto-generated record" for note in notes])) - - # == Test creating judicial portfolios == # - - # Clear old portfolios - Portfolio.objects.all().delete() - - expected_portfolio_names = { - self.judicial_agency_1.agency, - self.judicial_agency_2.agency, - } - self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True) - - # Ensure that all the portfolios we expect to get created were created - portfolios = Portfolio.objects.all() - self.assertEqual(portfolios.count(), 2) - - # Test that all created portfolios have the correct values - org_names, org_types, creators, notes = [], [], [], [] - for portfolio in portfolios: - org_names.append(portfolio.organization_name) - org_types.append(portfolio.organization_type) - creators.append(portfolio.creator) - notes.append(portfolio.notes) - - # Test organization_name, organization_type, creator, and notes (in that order) - self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names])) - self.assertTrue(all([org_type == federal_choice for org_type in org_types])) - self.assertTrue(all([creator == User.get_default_user() for creator in creators])) - self.assertTrue(all([note == "Auto-generated record" for note in notes])) def test_handle_portfolio_requests(self): """Verify portfolio association with domain requests.""" @@ -1662,23 +1681,19 @@ class TestCreateFederalPortfolio(TestCase): def test_command_error_parse_options(self): """Verify error when bad parse options are provided.""" # The command should enforce either --branch or --agency_name - with self.assertRaisesRegex( - CommandError, "Error: one of the arguments --agency_name --branch is required" - ): + with self.assertRaisesRegex(CommandError, "Error: one of the arguments --agency_name --branch is required"): self.run_create_federal_portfolio() - + # We should forbid both at the same time - with self.assertRaisesRegex( - CommandError, "Error: argument --branch: not allowed with argument --agency_name" - ): + with self.assertRaisesRegex(CommandError, "Error: argument --branch: not allowed with argument --agency_name"): self.run_create_federal_portfolio(agency_name="test", branch="executive") - + # We expect a error to be thrown when we dont pass parse requests or domains with self.assertRaisesRegex( CommandError, "You must specify at least one of --parse_requests or --parse_domains." ): self.run_create_federal_portfolio(branch="executive") - + with self.assertRaisesRegex( CommandError, "You must specify at least one of --parse_requests or --parse_domains." ): From efc4e71625567cc65927215491a95ed8f2460586 Mon Sep 17 00:00:00 2001 From: lizpearl Date: Wed, 27 Nov 2024 14:56:16 -0600 Subject: [PATCH 31/78] Change action needed reason from 'Already has domains' to 'Already has a domain' --- src/registrar/models/domain_request.py | 2 +- ...eady_has_domains.txt => already_has_a_domain.txt} | 0 src/registrar/tests/test_admin_request.py | 12 ++++++------ 3 files changed, 7 insertions(+), 7 deletions(-) rename src/registrar/templates/emails/action_needed_reasons/{already_has_domains.txt => already_has_a_domain.txt} (100%) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 0d8bbd5cf..b132ad5ac 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -280,7 +280,7 @@ class DomainRequest(TimeStampedModel): ELIGIBILITY_UNCLEAR = ("eligibility_unclear", "Unclear organization eligibility") QUESTIONABLE_SENIOR_OFFICIAL = ("questionable_senior_official", "Questionable senior official") - ALREADY_HAS_DOMAINS = ("already_has_domains", "Already has domains") + ALREADY_HAS_A_DOMAIN = ("already_has_a_domain", "Already has a domain") BAD_NAME = ("bad_name", "Doesn’t meet naming requirements") OTHER = ("other", "Other (no auto-email sent)") diff --git a/src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt b/src/registrar/templates/emails/action_needed_reasons/already_has_a_domain.txt similarity index 100% rename from src/registrar/templates/emails/action_needed_reasons/already_has_domains.txt rename to src/registrar/templates/emails/action_needed_reasons/already_has_a_domain.txt diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 9244fffcd..35912bed6 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -203,7 +203,7 @@ class TestDomainRequestAdmin(MockEppLib): domain_request.save() domain_request.action_needed() - domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS + domain_request.action_needed_reason = DomainRequest.ActionNeededReasons.ALREADY_HAS_A_DOMAIN domain_request.save() # Let's just change the action needed reason @@ -230,7 +230,7 @@ class TestDomainRequestAdmin(MockEppLib): "In review", "Rejected - Purpose requirements not met", "Action needed - Unclear organization eligibility", - "Action needed - Already has domains", + "Action needed - Already has a domain", "In review", "Submitted", "Started", @@ -241,7 +241,7 @@ class TestDomainRequestAdmin(MockEppLib): assert_status_count(normalized_content, "Started", 1) assert_status_count(normalized_content, "Submitted", 1) assert_status_count(normalized_content, "In review", 2) - assert_status_count(normalized_content, "Action needed - Already has domains", 1) + assert_status_count(normalized_content, "Action needed - Already has a domain", 1) assert_status_count(normalized_content, "Action needed - Unclear organization eligibility", 1) assert_status_count(normalized_content, "Rejected - Purpose requirements not met", 1) @@ -685,9 +685,9 @@ class TestDomainRequestAdmin(MockEppLib): # Create a sample domain request domain_request = completed_domain_request(status=in_review, user=_creator) - # Test the email sent out for already_has_domains - already_has_domains = DomainRequest.ActionNeededReasons.ALREADY_HAS_DOMAINS - self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=already_has_domains) + # Test the email sent out for already_has_a_domain + already_has_a_domain = DomainRequest.ActionNeededReasons.ALREADY_HAS_A_DOMAIN + self.transition_state_and_send_email(domain_request, action_needed, action_needed_reason=already_has_a_domain) self.assert_email_is_accurate("ORGANIZATION ALREADY HAS A .GOV DOMAIN", 0, EMAIL, bcc_email_address=BCC_EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) From 0c3fb834ff38928b41cdb9a77c72534f78286418 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:20:48 -0700 Subject: [PATCH 32/78] linting --- .../commands/create_federal_portfolio.py | 29 ++++++++++++------- .../tests/test_management_scripts.py | 2 +- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9e2598595..9b8541b75 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -82,15 +82,8 @@ class Command(BaseCommand): message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: - portfolio, created = self.create_portfolio(federal_agency) - if created: - self.create_suborganizations(portfolio, federal_agency) - if parse_domains or both: - self.handle_portfolio_domains(portfolio, federal_agency) - - if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) - + # C901 'Command.handle' is too complex (12) + self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -106,7 +99,21 @@ class Command(BaseCommand): display_as_str=True, ) + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): + """Attempts to create a portfolio. If successful, this function will + also create new suborganizations""" + portfolio, created = self.create_portfolio(federal_agency) + if created: + self.create_suborganizations(portfolio, federal_agency) + if parse_domains or both: + self.handle_portfolio_domains(portfolio, federal_agency) + + if parse_requests or both: + self.handle_portfolio_requests(portfolio, federal_agency) + def create_portfolio(self, federal_agency): + """Creates a portfolio if it doesn't presently exist. + Returns portfolio, created.""" # Get the org name / senior official org_name = federal_agency.agency so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None @@ -164,11 +171,11 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.warning, TerminalColors.FAIL, message) return - # Check if we need to update any existing suborgs first. This step is optional. + # Check for existing suborgs on the current portfolio existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): message = f"Some suborganizations already exist for portfolio '{portfolio}'." - TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 429aed0d9..7cce0d2b2 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1705,7 +1705,7 @@ class TestCreateFederalPortfolio(TestCase): "Cannot find the federal agency 'Non-existent Agency' in our database. " "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding." ) - with self.assertRaisesRegex(ValueError, expected_message): + with self.assertRaisesRegex(CommandError, expected_message): self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True) def test_does_not_update_existing_portfolio(self): From e5379d950040c5a20f30ae6fe79d1653ecf01bf7 Mon Sep 17 00:00:00 2001 From: lizpearl Date: Wed, 27 Nov 2024 15:25:53 -0600 Subject: [PATCH 33/78] Commit migration after field was renamed --- ...lter_domainrequest_action_needed_reason.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 src/registrar/migrations/0139_alter_domainrequest_action_needed_reason.py diff --git a/src/registrar/migrations/0139_alter_domainrequest_action_needed_reason.py b/src/registrar/migrations/0139_alter_domainrequest_action_needed_reason.py new file mode 100644 index 000000000..c3af6905e --- /dev/null +++ b/src/registrar/migrations/0139_alter_domainrequest_action_needed_reason.py @@ -0,0 +1,28 @@ +# Generated by Django 4.2.10 on 2024-11-27 21:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0138_alter_domaininvitation_status"), + ] + + operations = [ + migrations.AlterField( + model_name="domainrequest", + name="action_needed_reason", + field=models.TextField( + blank=True, + choices=[ + ("eligibility_unclear", "Unclear organization eligibility"), + ("questionable_senior_official", "Questionable senior official"), + ("already_has_a_domain", "Already has a domain"), + ("bad_name", "Doesn’t meet naming requirements"), + ("other", "Other (no auto-email sent)"), + ], + null=True, + ), + ), + ] From 8304caf026b1b5a7be75bbd748c0584c41af5c3d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 27 Nov 2024 14:33:13 -0700 Subject: [PATCH 34/78] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9b8541b75..64672366b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -87,7 +87,7 @@ class Command(BaseCommand): except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) - message = f"Failed to create portfolio '{portfolio}'" + message = f"Failed to create portfolio '{federal_agency.agency}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message) TerminalHelper.log_script_run_summary( @@ -174,6 +174,10 @@ class Command(BaseCommand): # Check for existing suborgs on the current portfolio existing_suborgs = Suborganization.objects.filter(name__in=org_names) if existing_suborgs.exists(): + # QUESTION FOR REVIEWERS: Should we be doing this too? + # existing_suborgs.filter(portfolio__isnull=True).update( + # portfolio=portfolio + # ) message = f"Some suborganizations already exist for portfolio '{portfolio}'." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) From 20da68978983c6742d0f5cfffa992f831518fab1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Nov 2024 23:25:20 -0500 Subject: [PATCH 35/78] tweak gitignore --- .gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b49b30639..5540ec869 100644 --- a/.gitignore +++ b/.gitignore @@ -3,7 +3,8 @@ docs/research/data/** **/assets/* !**/assets/src/ -!**/assets/sass/ +!**/assets/src/js/ +!**/assets/src/sass/ !**/assets/img/registrar/ public/ credentials* From 84f85d944f20d621ccdd9b4f4c0d9c46ba548812 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Nov 2024 14:14:23 -0700 Subject: [PATCH 36/78] Do the thing --- src/registrar/forms/domain_request_wizard.py | 15 +++++++++++++++ ...rtfolio_domain_request_additional_details.html | 10 +++++----- src/registrar/views/domain_request.py | 2 +- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index e55c40858..5a791e921 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -794,6 +794,21 @@ class AnythingElseForm(BaseDeletableRegistrarForm): ) +class PortfolioAnythingElseForm(BaseDeletableRegistrarForm): + """The form for the portfolio additional details page. Tied to the anything_else field.""" + anything_else = forms.CharField( + required=False, + label="Anything else?", + widget=forms.Textarea(), + validators=[ + MaxLengthValidator( + 2000, + message="Response must be less than 2000 characters.", + ) + ], + ) + + class AnythingElseYesNoForm(BaseYesNoForm): """Yes/no toggle for the anything else question on additional details""" diff --git a/src/registrar/templates/portfolio_domain_request_additional_details.html b/src/registrar/templates/portfolio_domain_request_additional_details.html index 3c5b50d6b..5bc529243 100644 --- a/src/registrar/templates/portfolio_domain_request_additional_details.html +++ b/src/registrar/templates/portfolio_domain_request_additional_details.html @@ -2,18 +2,18 @@ {% load static field_helpers %} {% block form_required_fields_help_text %} -{% include "includes/required_fields.html" %} +{% comment %} Empty - this step is not required {% endcomment %} {% endblock %} {% block form_fields %} -
-

Is there anything else you’d like us to know about your domain request?

+
+

Is there anything else you’d like us to know about your domain request?

-
-

Provide details below. *

+
+

This question is optional.

{% with attr_maxlength=2000 add_label_class="usa-sr-only" %} {% input_with_errors forms.0.anything_else %} {% endwith %} diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index da194755f..f52c29e9a 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -614,7 +614,7 @@ class RequestingEntity(DomainRequestWizard): class PortfolioAdditionalDetails(DomainRequestWizard): template_name = "portfolio_domain_request_additional_details.html" - forms = [forms.AnythingElseForm] + forms = [forms.PortfolioAnythingElseForm] # Non-portfolio pages From 6ca53560eadc63105eda00edd20305f370049240 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Fri, 29 Nov 2024 15:17:09 -0700 Subject: [PATCH 37/78] 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 @@ class BaseExport(ABC): @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 38/78] 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 @@ class TestDomainAdminWithClient(TestCase): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.maxDiff = None self.assertEqual(csv_content, expected_content) @@ -509,7 +508,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): 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 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.assertEqual(csv_content, expected_content) @less_console_noise_decorator @@ -727,7 +724,6 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib): # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - self.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 @@ class BaseExport(ABC): 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 @@ class DomainExport(BaseExport): 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 @@ class DomainExport(BaseExport): "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 @@ class DomainRequestExport(BaseExport): ) @classmethod - def get_computed_fields(cls, delimiter=", "): + def get_computed_fields(cls, delimiter=", ", **kwargs): """ Get a dict of computed fields. """ @@ -1701,7 +1701,7 @@ class DomainRequestExport(BaseExport): "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 d5357bc2eeb1c6ce319b1069d16c99d70b148fb4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:19:35 -0700 Subject: [PATCH 39/78] Code cleanup --- src/registrar/config/urls.py | 4 ++-- .../templates/includes/portfolio_request_review_steps.html | 2 +- src/registrar/views/domain_request.py | 6 ++++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 53b83e564..66a8a9b74 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -46,8 +46,8 @@ DOMAIN_REQUEST_NAMESPACE = views.DomainRequestWizard.URL_NAMESPACE # dynamically generate the other domain_request_urls domain_request_urls = [ path("", RedirectView.as_view(pattern_name="domain-request:start"), name="redirect-to-start"), - path("start/", views.DomainRequestWizard.as_view(), name="start"), - path("finished/", views.Finished.as_view(), name="finished"), + path("start/", views.DomainRequestWizard.as_view(), name=views.DomainRequestWizard.NEW_URL_NAME), + path("finished/", views.Finished.as_view(), name=views.DomainRequestWizard.FINISHED_URL_NAME), ] for step, view in [ # add/remove steps here diff --git a/src/registrar/templates/includes/portfolio_request_review_steps.html b/src/registrar/templates/includes/portfolio_request_review_steps.html index 5c6e64269..fcb087090 100644 --- a/src/registrar/templates/includes/portfolio_request_review_steps.html +++ b/src/registrar/templates/includes/portfolio_request_review_steps.html @@ -62,7 +62,7 @@ {% endif %} {% if step == Step.ADDITIONAL_DETAILS %} - {% with title=form_titles|get_item:step value=domain_request.anything_else|default:"Incomplete"|safe %} + {% with title=form_titles|get_item:step value=domain_request.anything_else|default:"None" %} {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url %} {% endwith %} {% endif %} diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index f52c29e9a..1fd835055 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -12,6 +12,7 @@ from registrar.forms.utility.wizard_form_helper import request_step_list from registrar.models import DomainRequest from registrar.models.contact import Contact from registrar.models.user import User +from registrar.models.utility.generic_helper import get_url_name from registrar.views.utility import StepsHelper from registrar.views.utility.permission_views import DomainRequestPermissionDeleteView from registrar.utility.enums import Step, PortfolioDomainRequestStep @@ -53,7 +54,8 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): URL_NAMESPACE = "domain-request" # name for accessing /domain-request//edit EDIT_URL_NAME = "edit-domain-request" - NEW_URL_NAME = "/request/start/" + NEW_URL_NAME = "start" + FINISHED_URL_NAME = "finish" # region: Titles # We need to pass our human-readable step titles as context to the templates. @@ -313,7 +315,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): # send users "to the domain request wizard" without needing to know which view # is first in the list of steps. if self.__class__ == DomainRequestWizard: - if request.path_info == self.NEW_URL_NAME: + if current_url == self.NEW_URL_NAME: # Clear context so the prop getter won't create a request here. # Creating a request will be handled in the post method for the # intro page. From ee0f5e8a0de5211fd58432b67ce37a36906f65fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:31:06 -0700 Subject: [PATCH 40/78] Cleanup --- src/registrar/forms/domain_request_wizard.py | 1 + src/registrar/views/domain_request.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 5a791e921..95d6571d6 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -796,6 +796,7 @@ class AnythingElseForm(BaseDeletableRegistrarForm): class PortfolioAnythingElseForm(BaseDeletableRegistrarForm): """The form for the portfolio additional details page. Tied to the anything_else field.""" + anything_else = forms.CharField( required=False, label="Anything else?", diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 1fd835055..33c157388 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -55,7 +55,7 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): # name for accessing /domain-request//edit EDIT_URL_NAME = "edit-domain-request" NEW_URL_NAME = "start" - FINISHED_URL_NAME = "finish" + FINISHED_URL_NAME = "finished" # region: Titles # We need to pass our human-readable step titles as context to the templates. From d5a8af228701a0b1ee12e1a75f70775f11da459c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Dec 2024 10:38:00 -0700 Subject: [PATCH 41/78] Update domain_request.py --- src/registrar/views/domain_request.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 33c157388..85f7576d0 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -12,7 +12,6 @@ from registrar.forms.utility.wizard_form_helper import request_step_list from registrar.models import DomainRequest from registrar.models.contact import Contact from registrar.models.user import User -from registrar.models.utility.generic_helper import get_url_name from registrar.views.utility import StepsHelper from registrar.views.utility.permission_views import DomainRequestPermissionDeleteView from registrar.utility.enums import Step, PortfolioDomainRequestStep From 366ecb97d94e93bd2af53e82bbe5cd2ca50b1581 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Dec 2024 14:07:15 -0700 Subject: [PATCH 42/78] basic logic --- src/registrar/config/settings.py | 6 +++ src/registrar/models/domain.py | 87 +++++++++++++++++++++++++++++++- src/registrar/views/domain.py | 6 +++ 3 files changed, 98 insertions(+), 1 deletion(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index a18a813f1..bcf4d79d6 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -86,6 +86,10 @@ secret_registry_key = b64decode(secret("REGISTRY_KEY", "")) secret_registry_key_passphrase = secret("REGISTRY_KEY_PASSPHRASE", "") secret_registry_hostname = secret("REGISTRY_HOSTNAME") +# PROTOTYPE: Used for DNS hosting +secret_registry_tenant_key = secret("REGISTRY_TENANT_KEY", None) +secret_registry_tenant_id = secret("REGISTRY_TENANT_ID", None) + # region: Basic Django Config-----------------------------------------------### # Build paths inside the project like this: BASE_DIR / "subdir". @@ -685,6 +689,8 @@ SECRET_REGISTRY_CERT = secret_registry_cert SECRET_REGISTRY_KEY = secret_registry_key SECRET_REGISTRY_KEY_PASSPHRASE = secret_registry_key_passphrase SECRET_REGISTRY_HOSTNAME = secret_registry_hostname +SECRET_REGISTRY_TENANT_KEY = secret_registry_tenant_key +SECRET_REGISTRY_TENANT_ID = secret_registry_tenant_id # endregion # region: Security and Privacy----------------------------------------------### diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 7fdc56971..2718a225e 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1,10 +1,11 @@ from itertools import zip_longest import logging import ipaddress +import requests import re from datetime import date from typing import Optional - +from django.conf import settings from django_fsm import FSMField, transition, TransitionNotAllowed # type: ignore from django.db import models @@ -307,6 +308,90 @@ class Domain(TimeStampedModel, DomainHelper): To update the expiration date, use renew_domain method.""" raise NotImplementedError() + def create_dns_record(self, dns_record_dict): + print(f"what is the key? {settings.SECRET_REGISTRY_TENANT_KEY}") + # Cloudflare API endpoints + base_url = "https://api.cloudflare.com/client/v4" + headers = { + "Authorization": f"Bearer {settings.SECRET_REGISTRY_TENANT_KEY}", + "Content-Type": "application/json" + } + if settings.IS_PRODUCTION: + if self.name == "igorville.gov": + # do stuff + pass + else: + logger.warning(f"create_dns_record was called for domain {self.name}") + else: + pass + + # TODO - check if these things exist before doing stuff + # 1. Get tenant details + # Note: we can grab this more generally but lets be specific to keep things safe. + tenant_id = settings.SECRET_REGISTRY_TENANT_ID + account_name = f"account-{self.name}" + + # 2. Create account under tenant + account_response = requests.post( + f"{base_url}/accounts", + headers=headers, + json={ + "name": account_name, + "type": "enterprise", + "unit": {"id": tenant_id} + } + ) + account_response.raise_for_status() + account_response_json = account_response.json() + account_id = account_response_json["result"]["id"] + logger.info(f"Created account: {account_response_json}") + + # 3. Create zone under account + zone_response = requests.post( + f"{base_url}/zones", + headers=headers, + json={ + "name": self.name, + "account": {"id": account_id}, + "type": "full" + } + ) + zone_response.raise_for_status() + zone_response_json = zone_response.json() + zone_id = zone_response_json["result"]["id"] + logger.info(f"Created zone: {zone_id}") + + # 4. Add zone subscription + subscription_response = requests.post( + f"{base_url}/zones/{zone_id}/subscription", + headers=headers, + json={ + "rate_plan": {"id": "PARTNERS_ENT"}, + "frequency": "annual" + } + ) + subscription_response.raise_for_status() + subscription_response_json = subscription_response.json() + logger.info(f"Created subscription: {subscription_response_json}") + + # 5. Create DNS record + dns_response = requests.post( + f"{base_url}/zones/{zone_id}/dns_records", + headers=headers, + json=dns_record_dict + ) + dns_response.raise_for_status() + dns_response_json = dns_response.json() + logger.info(f"Created DNS record: {dns_response_json}") + + return { + "tenant_id": tenant_id, + "account_id": account_id, + "zone_id": zone_id, + "dns_record_id": dns_response_json["result"]["id"] + } + + def renew_domain(self, length: int = 1, unit: epp.Unit = epp.Unit.YEAR): """ Renew the domain to a length and unit of time relative to the current diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 9bf6f5313..b65cd93be 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -455,6 +455,12 @@ class DomainDNSView(DomainBaseView): template_name = "domain_dns.html" + def get_context_data(self, **kwargs): + """Adds custom context.""" + context = super().get_context_data(**kwargs) + context["dns_prototype_flag"] = flag_is_active_for_user(self.request.user, "dns_prototype_flag") + return context + class DomainNameserversView(DomainFormBaseView): """Domain nameserver editing view.""" From 3fc9cf31e9940b5f43340e785f0b8d48f6fba2bc Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 3 Dec 2024 16:21:04 -0500 Subject: [PATCH 43/78] Remove stray JS from org contact template --- src/registrar/templates/domain_request_org_contact.html | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/templates/domain_request_org_contact.html b/src/registrar/templates/domain_request_org_contact.html index d39fb9f78..d9f39d6fe 100644 --- a/src/registrar/templates/domain_request_org_contact.html +++ b/src/registrar/templates/domain_request_org_contact.html @@ -43,6 +43,3 @@
{% endblock %} - - - From f7bbed1f03950bbb06871267a56cff6abdd97e0b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 3 Dec 2024 16:25:49 -0500 Subject: [PATCH 44/78] remove inline CSS --- src/registrar/templates/domain_request_org_contact.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_request_org_contact.html b/src/registrar/templates/domain_request_org_contact.html index d9f39d6fe..280d507a9 100644 --- a/src/registrar/templates/domain_request_org_contact.html +++ b/src/registrar/templates/domain_request_org_contact.html @@ -37,7 +37,7 @@ {% input_with_errors forms.0.zipcode %} {% endwith %} -