From 44487b40b2116b8bde626a7f6785c678a14de605 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 27 Mar 2024 17:38:22 -0400 Subject: [PATCH 01/13] wip --- src/registrar/utility/csv_export.py | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 7fc710827..22d2fc46d 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -3,6 +3,7 @@ import csv import logging from datetime import datetime from registrar.models.domain import Domain +from registrar.models.domain_invitation import DomainInvitation from registrar.models.domain_request import DomainRequest from registrar.models.domain_information import DomainInformation from django.utils import timezone @@ -33,7 +34,7 @@ def get_domain_infos(filter_condition, sort_fields): """ domain_infos = ( DomainInformation.objects.select_related("domain", "authorizing_official") - .prefetch_related("domain__permissions") + .prefetch_related("domain__permissions", "domain__invitations") .filter(**filter_condition) .order_by(*sort_fields) .distinct() @@ -154,6 +155,10 @@ def write_domains_csv( all_domain_infos = get_domain_infos(filter_condition, sort_fields) + # td_agencies = all_domain_infos.filter(domain__invitation__status='invited').annotate(invitations_count=Count('invitations')).values_list('domain_name', 'invitations_count').distinct() + # Create a dictionary mapping of domain_name to federal_agency + # td_dict = dict(td_agencies) + # Store all security emails to avoid epp calls or excessive filters sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) @@ -164,6 +169,8 @@ def write_domains_csv( # The maximum amount of domain managers an account has # We get the max so we can set the column header accurately + max_dm_active = 0 + max_dm_invited = 0 max_dm_count = 0 total_body_rows = [] @@ -172,15 +179,26 @@ def write_domains_csv( page = paginator.page(page_num) for domain_info in page.object_list: - # Get count of all the domain managers for an account + # Get max number of domain managers if get_domain_managers: - dm_count = domain_info.domain.permissions.count() - if dm_count > max_dm_count: - max_dm_count = dm_count + dm_active = domain_info.domain.permissions.count() + if dm_active > max_dm_active: + max_dm_active = dm_active + + # Now let's get the domain managers who have not retrieved their invite yet + # Let's get the max number of whose + dm_invited = domain_info.domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).count() + if dm_invited > max_dm_invited: + max_dm_invited = dm_invited + + if dm_active > max_dm_active or dm_invited > max_dm_invited: + max_dm_count = max_dm_active + max_dm_invited for i in range(1, max_dm_count + 1): column_name = f"Domain manager email {i}" + column2_name = f"DM{i} status" if column_name not in columns: columns.append(column_name) + columns.append(column2_name) try: row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers) From 5cd70dad349ffafc9154a1dea8fcf8cf648a5c8e Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 28 Mar 2024 19:03:00 -0400 Subject: [PATCH 02/13] add invited DMs to reports --- src/registrar/tests/common.py | 19 ++++++++++ src/registrar/tests/test_reports.py | 35 +++++++++++------- src/registrar/utility/csv_export.py | 55 ++++++++++++++++++----------- 3 files changed, 77 insertions(+), 32 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 87af54669..a4d16387e 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -669,6 +669,24 @@ class MockDb(TestCase): user=meoward_user, domain=self.domain_12, role=UserDomainRole.Roles.MANAGER ) + _, created = DomainInvitation.objects.get_or_create( + email=meoward_user.email, domain=self.domain_1, status=DomainInvitation.DomainInvitationStatus.RETRIEVED + ) + + _, created = DomainInvitation.objects.get_or_create( + email="woofwardthethird@rocks.com", + domain=self.domain_1, + status=DomainInvitation.DomainInvitationStatus.INVITED, + ) + + _, created = DomainInvitation.objects.get_or_create( + email="squeaker@rocks.com", domain=self.domain_2, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + + _, created = DomainInvitation.objects.get_or_create( + email="squeaker@rocks.com", domain=self.domain_10, status=DomainInvitation.DomainInvitationStatus.INVITED + ) + with less_console_noise(): self.domain_request_1 = completed_domain_request( status=DomainRequest.DomainRequestStatus.STARTED, name="city1.gov" @@ -698,6 +716,7 @@ class MockDb(TestCase): DomainRequest.objects.all().delete() User.objects.all().delete() UserDomainRole.objects.all().delete() + DomainInvitation.objects.all().delete() def mock_user(): diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 5bd594a15..cd882c4f8 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -478,7 +478,12 @@ class ExportDataTest(MockDb, MockEppLib): def test_export_domains_to_writer_domain_managers(self): """Test that export_domains_to_writer returns the - expected domain managers.""" + expected domain managers. + + An invited user, woofwardthethird, should also be pulled into this report. + + squeaker@rocks.com is invited to domain2 (DNS_NEEDED) and domain10 (No managers). + She should show twice in this report but not in test_export_data_managed_domains_to_csv.""" with less_console_noise(): # Create a CSV file in memory @@ -521,14 +526,16 @@ class ExportDataTest(MockDb, MockEppLib): expected_content = ( "Domain name,Status,Expiration date,Domain type,Agency," "Organization name,City,State,AO,AO email," - "Security contact email,Domain manager email 1,Domain manager email 2,Domain manager email 3\n" - "adomain10.gov,Ready,,Federal,Armed Forces Retirement Home,,,, , ,\n" - "adomain2.gov,Dns needed,,Interstate,,,,, , , ,meoward@rocks.com\n" - "cdomain11.govReadyFederal-ExecutiveWorldWarICentennialCommissionmeoward@rocks.com\n" + "Security contact email,Domain manager 1,DM1 status,Domain manager 2,DM2 status," + "Domain manager 3,DM3 status,Domain manager 4,DM4 status\n" + "adomain10.gov,Ready,,Federal,Armed Forces Retirement Home,,,, , ,squeaker@rocks.com, I\n" + "adomain2.gov,Dns needed,,Interstate,,,,, , , ,meoward@rocks.com, R,squeaker@rocks.com, I\n" + "cdomain11.govReadyFederal-ExecutiveWorldWarICentennialCommissionmeoward@rocks.comR\n" "cdomain1.gov,Ready,,Federal - Executive,World War I Centennial Commission,,," - ", , , ,meoward@rocks.com,info@example.com,big_lebowski@dude.co\n" + ", , , ,meoward@rocks.com,R,info@example.com,R,big_lebowski@dude.co,R," + "woofwardthethird@rocks.com,I\n" "ddomain3.gov,On hold,,Federal,Armed Forces Retirement Home,,,, , , ,,\n" - "zdomain12.govReadyInterstatemeoward@rocks.com\n" + "zdomain12.govReadyInterstatemeoward@rocks.comR\n" ) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace @@ -538,7 +545,9 @@ class ExportDataTest(MockDb, MockEppLib): def test_export_data_managed_domains_to_csv(self): """Test get counts for domains that have domain managers for two different dates, - get list of managed domains at end_date.""" + get list of managed domains at end_date. + + An invited user, woofwardthethird, should also be pulled into this report.""" with less_console_noise(): # Create a CSV file in memory @@ -564,10 +573,12 @@ class ExportDataTest(MockDb, MockEppLib): "Special district,School district,Election office\n" "3,2,1,0,0,0,0,0,0,2\n" "\n" - "Domain name,Domain type,Domain manager email 1,Domain manager email 2,Domain manager email 3\n" - "cdomain11.govFederal-Executivemeoward@rocks.com\n" - "cdomain1.gov,Federal - Executive,meoward@rocks.com,info@example.com,big_lebowski@dude.co\n" - "zdomain12.govInterstatemeoward@rocks.com\n" + "Domain name,Domain type,Domain manager 1,DM1 status,Domain manager 2,DM2 status," + "Domain manager 3,DM3 status,Domain manager 4,DM4 status\n" + "cdomain11.govFederal-Executivemeoward@rocks.com, R\n" + "cdomain1.gov,Federal - Executive,meoward@rocks.com,R,info@example.com,R," + "big_lebowski@dude.co,R,woofwardthethird@rocks.com,I\n" + "zdomain12.govInterstatemeoward@rocks.com,R\n" ) # Normalize line endings and remove commas, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 22d2fc46d..d0f81fc45 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -105,12 +105,22 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di } if get_domain_managers: - # Get each domain managers email and add to list - dm_emails = [dm.user.email for dm in domain.permissions.all()] + # Get lists of emails for active and invited domain managers + dm_active_emails = [dm.user.email for dm in domain.permissions.all()] + dm_invited_emails = [ + invite.email for invite in domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) + ] - # Set up the "matching header" + row field data - for i, dm_email in enumerate(dm_emails, start=1): - FIELDS[f"Domain manager email {i}"] = dm_email + # Set up the "matching headers" + row field data for email and status + i = 0 # Declare i outside of the loop to avoid a reference before assignment in the second loop + for i, dm_email in enumerate(dm_active_emails, start=1): + FIELDS[f"Domain manager {i}"] = dm_email + FIELDS[f"DM{i} status"] = "R" + + # Continue enumeration from where we left off and add data for invited domain managers + for j, dm_email in enumerate(dm_invited_emails, start=i + 1): + FIELDS[f"Domain manager {j}"] = dm_email + FIELDS[f"DM{j} status"] = "I" row = [FIELDS.get(column, "") for column in columns] return row @@ -158,7 +168,7 @@ def write_domains_csv( # td_agencies = all_domain_infos.filter(domain__invitation__status='invited').annotate(invitations_count=Count('invitations')).values_list('domain_name', 'invitations_count').distinct() # Create a dictionary mapping of domain_name to federal_agency # td_dict = dict(td_agencies) - + # Store all security emails to avoid epp calls or excessive filters sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) @@ -167,11 +177,15 @@ def write_domains_csv( # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) - # The maximum amount of domain managers an account has - # We get the max so we can set the column header accurately + # We get the number of domain managers (DMs) an the domain + # that has the most DMs so we can set the header row appropriately max_dm_active = 0 max_dm_invited = 0 - max_dm_count = 0 + max_dm_total = 0 + update_columns = False + + # This var will live outside of the nested for loops to aggregate + # the data from those loops total_body_rows = [] for page_num in paginator.page_range: @@ -182,23 +196,24 @@ def write_domains_csv( # Get max number of domain managers if get_domain_managers: dm_active = domain_info.domain.permissions.count() - if dm_active > max_dm_active: - max_dm_active = dm_active - - # Now let's get the domain managers who have not retrieved their invite yet - # Let's get the max number of whose - dm_invited = domain_info.domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).count() - if dm_invited > max_dm_invited: - max_dm_invited = dm_invited + dm_invited = domain_info.domain.invitations.filter( + status=DomainInvitation.DomainInvitationStatus.INVITED + ).count() if dm_active > max_dm_active or dm_invited > max_dm_invited: - max_dm_count = max_dm_active + max_dm_invited - for i in range(1, max_dm_count + 1): - column_name = f"Domain manager email {i}" + max_dm_active = max(dm_active, max_dm_active) + max_dm_invited = max(dm_invited, max_dm_invited) + max_dm_total = max_dm_active + max_dm_invited + update_columns = True + + if update_columns: + for i in range(1, max_dm_total + 1): + column_name = f"Domain manager {i}" column2_name = f"DM{i} status" if column_name not in columns: columns.append(column_name) columns.append(column2_name) + update_columns = False try: row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers) From 7aefce86db56e97fe6a3adb4053b1f8608d16d9d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 28 Mar 2024 19:33:19 -0400 Subject: [PATCH 03/13] Lint --- src/registrar/utility/csv_export.py | 63 ++++++++++++++++++----------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d0f81fc45..98e3786c5 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -148,6 +148,41 @@ def _get_security_emails(sec_contact_ids): return security_emails_dict +def update_columns_with_domain_managers( + domain_info, update_columns, columns, max_dm_active, max_dm_invited, max_dm_total +): + """Helper function that works with 'global' variables set in write_domains_csv + Accepts: + domain_info -> Domains to parse + update_columns -> A control to make sure we only run the columns test and update when needed + columns -> The header cells in the csv that's under construction + max_dm_active -> Starts at 0 and gets updated and passed again through this method + max_dm_invited -> Starts at 0 and gets updated and passed again through this method + max_dm_total -> Starts at 0 and gets updated and passed again through this method + Returns: + Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" + + dm_active = domain_info.domain.permissions.count() + dm_invited = domain_info.domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).count() + + if dm_active > max_dm_active or dm_invited > max_dm_invited: + max_dm_active = max(dm_active, max_dm_active) + max_dm_invited = max(dm_invited, max_dm_invited) + max_dm_total = max_dm_active + max_dm_invited + update_columns = True + + if update_columns: + for i in range(1, max_dm_total + 1): + column_name = f"Domain manager {i}" + column2_name = f"DM{i} status" + if column_name not in columns: + columns.append(column_name) + columns.append(column2_name) + update_columns = False + + return update_columns, columns, max_dm_active, max_dm_invited, max_dm_total + + def write_domains_csv( writer, columns, @@ -165,10 +200,6 @@ def write_domains_csv( all_domain_infos = get_domain_infos(filter_condition, sort_fields) - # td_agencies = all_domain_infos.filter(domain__invitation__status='invited').annotate(invitations_count=Count('invitations')).values_list('domain_name', 'invitations_count').distinct() - # Create a dictionary mapping of domain_name to federal_agency - # td_dict = dict(td_agencies) - # Store all security emails to avoid epp calls or excessive filters sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) @@ -195,25 +226,11 @@ def write_domains_csv( # Get max number of domain managers if get_domain_managers: - dm_active = domain_info.domain.permissions.count() - dm_invited = domain_info.domain.invitations.filter( - status=DomainInvitation.DomainInvitationStatus.INVITED - ).count() - - if dm_active > max_dm_active or dm_invited > max_dm_invited: - max_dm_active = max(dm_active, max_dm_active) - max_dm_invited = max(dm_invited, max_dm_invited) - max_dm_total = max_dm_active + max_dm_invited - update_columns = True - - if update_columns: - for i in range(1, max_dm_total + 1): - column_name = f"Domain manager {i}" - column2_name = f"DM{i} status" - if column_name not in columns: - columns.append(column_name) - columns.append(column2_name) - update_columns = False + update_columns, columns, max_dm_active, max_dm_invited, max_dm_total = ( + update_columns_with_domain_managers( + domain_info, update_columns, columns, max_dm_active, max_dm_invited, max_dm_total + ) + ) try: row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers) From 63953a2471619b97a2c1ab54df622fada0fdd394 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Mar 2024 14:40:27 -0400 Subject: [PATCH 04/13] tweak update_columns_with_domain_managers --- src/registrar/utility/csv_export.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 98e3786c5..5db51b539 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -165,13 +165,16 @@ def update_columns_with_domain_managers( dm_active = domain_info.domain.permissions.count() dm_invited = domain_info.domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).count() - if dm_active > max_dm_active or dm_invited > max_dm_invited: + if dm_active > max_dm_active: max_dm_active = max(dm_active, max_dm_active) + update_columns = True + + if dm_invited > max_dm_invited: max_dm_invited = max(dm_invited, max_dm_invited) - max_dm_total = max_dm_active + max_dm_invited update_columns = True if update_columns: + max_dm_total = max_dm_active + max_dm_invited for i in range(1, max_dm_total + 1): column_name = f"Domain manager {i}" column2_name = f"DM{i} status" From 8c8d7b6b63914234981b7a85200c547b7a3a4bcf Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 29 Mar 2024 16:32:51 -0400 Subject: [PATCH 05/13] Fix logic, optimize analytics page load --- src/registrar/assets/js/get-gov-reports.js | 4 +- src/registrar/templates/admin/analytics.html | 4 +- src/registrar/utility/csv_export.py | 146 +++++++------------ src/registrar/views/admin_views.py | 52 +++---- 4 files changed, 84 insertions(+), 122 deletions(-) diff --git a/src/registrar/assets/js/get-gov-reports.js b/src/registrar/assets/js/get-gov-reports.js index da0781411..92bba4a1f 100644 --- a/src/registrar/assets/js/get-gov-reports.js +++ b/src/registrar/assets/js/get-gov-reports.js @@ -117,8 +117,8 @@ function initComparativeColumnCharts() { document.addEventListener("DOMContentLoaded", function () { - // createComparativeColumnChart("myChart1", "Managed domains", "Start Date", "End Date"); - // createComparativeColumnChart("myChart2", "Unmanaged domains", "Start Date", "End Date"); + createComparativeColumnChart("myChart1", "Managed domains", "Start Date", "End Date"); + createComparativeColumnChart("myChart2", "Unmanaged domains", "Start Date", "End Date"); createComparativeColumnChart("myChart3", "Deleted domains", "Start Date", "End Date"); createComparativeColumnChart("myChart4", "Ready domains", "Start Date", "End Date"); createComparativeColumnChart("myChart5", "Submitted requests", "Start Date", "End Date"); diff --git a/src/registrar/templates/admin/analytics.html b/src/registrar/templates/admin/analytics.html index 6812c919d..e73f22ec5 100644 --- a/src/registrar/templates/admin/analytics.html +++ b/src/registrar/templates/admin/analytics.html @@ -113,7 +113,7 @@ - +
diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 5db51b539..0521a71be 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -1,4 +1,3 @@ -from collections import Counter import csv import logging from datetime import datetime @@ -8,7 +7,7 @@ from registrar.models.domain_request import DomainRequest from registrar.models.domain_information import DomainInformation from django.utils import timezone from django.core.paginator import Paginator -from django.db.models import F, Value, CharField +from django.db.models import F, Value, CharField, Q, Count from django.db.models.functions import Concat, Coalesce from registrar.models.public_contact import PublicContact @@ -54,7 +53,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_domain_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False): +def parse_domain_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False, invites_with_invited_status=None): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -108,7 +107,7 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di # Get lists of emails for active and invited domain managers dm_active_emails = [dm.user.email for dm in domain.permissions.all()] dm_invited_emails = [ - invite.email for invite in domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED) + invite.email for invite in invites_with_invited_status.filter(domain=domain) ] # Set up the "matching headers" + row field data for email and status @@ -149,32 +148,25 @@ def _get_security_emails(sec_contact_ids): def update_columns_with_domain_managers( - domain_info, update_columns, columns, max_dm_active, max_dm_invited, max_dm_total + domain_info,invites_with_invited_status, update_columns, columns, max_dm_total ): """Helper function that works with 'global' variables set in write_domains_csv Accepts: domain_info -> Domains to parse update_columns -> A control to make sure we only run the columns test and update when needed columns -> The header cells in the csv that's under construction - max_dm_active -> Starts at 0 and gets updated and passed again through this method - max_dm_invited -> Starts at 0 and gets updated and passed again through this method max_dm_total -> Starts at 0 and gets updated and passed again through this method Returns: Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" dm_active = domain_info.domain.permissions.count() - dm_invited = domain_info.domain.invitations.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).count() + dm_invited = invites_with_invited_status.filter(domain=domain_info.domain).count() - if dm_active > max_dm_active: - max_dm_active = max(dm_active, max_dm_active) - update_columns = True - - if dm_invited > max_dm_invited: - max_dm_invited = max(dm_invited, max_dm_invited) + if dm_active + dm_invited > max_dm_total: + max_dm_total = dm_active + dm_invited update_columns = True if update_columns: - max_dm_total = max_dm_active + max_dm_invited for i in range(1, max_dm_total + 1): column_name = f"Domain manager {i}" column2_name = f"DM{i} status" @@ -183,7 +175,7 @@ def update_columns_with_domain_managers( columns.append(column2_name) update_columns = False - return update_columns, columns, max_dm_active, max_dm_invited, max_dm_total + return update_columns, columns, max_dm_total def write_domains_csv( @@ -213,10 +205,18 @@ def write_domains_csv( # We get the number of domain managers (DMs) an the domain # that has the most DMs so we can set the header row appropriately - max_dm_active = 0 - max_dm_invited = 0 + max_dm_total = 0 update_columns = False + invites_with_invited_status=None + + if get_domain_managers: + invites_with_invited_status = DomainInvitation.objects.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).prefetch_related("domain") + + # zander = DomainInformation.objects.filter(**filter_condition).annotate(invitations_count=Count('invitation', filter=Q(invitation__status='invited'))).values_list('domain_name', 'invitations_count') + # logger.info(f'zander {zander}') + # zander_dict = dict(zander) + # logger.info(f'zander_dict {zander_dict}') # This var will live outside of the nested for loops to aggregate # the data from those loops @@ -229,14 +229,14 @@ def write_domains_csv( # Get max number of domain managers if get_domain_managers: - update_columns, columns, max_dm_active, max_dm_invited, max_dm_total = ( + update_columns, columns, max_dm_total = ( update_columns_with_domain_managers( - domain_info, update_columns, columns, max_dm_active, max_dm_invited, max_dm_total + domain_info,invites_with_invited_status, update_columns, columns, max_dm_total ) ) try: - row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers) + row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers, invites_with_invited_status) rows.append(row) except ValueError: # This should not happen. If it does, just skip this row. @@ -518,58 +518,23 @@ def get_sliced_domains(filter_condition, distinct=False): when a domain has more that one manager. """ - # Round trip 1: Get distinct domain names based on filter condition - domains_count = DomainInformation.objects.filter(**filter_condition).distinct().count() - - # Round trip 2: Get counts for other slices - # This will require either 8 filterd and distinct DB round trips, - # or 2 DB round trips plus iteration on domain_permissions for each domain - if distinct: - generic_org_types_query = DomainInformation.objects.filter(**filter_condition).values_list( - "domain_id", "generic_org_type" - ) - # Initialize Counter to store counts for each generic_org_type - generic_org_type_counts = Counter() - - # Keep track of domains already counted - domains_counted = set() - - # Iterate over distinct domains - for domain_id, generic_org_type in generic_org_types_query: - # Check if the domain has already been counted - if domain_id in domains_counted: - continue - - # Get all permissions for the current domain - domain_permissions = DomainInformation.objects.filter(domain_id=domain_id, **filter_condition).values_list( - "domain__permissions", flat=True - ) - - # Check if the domain has multiple permissions - if len(domain_permissions) > 0: - # Mark the domain as counted - domains_counted.add(domain_id) - - # Increment the count for the corresponding generic_org_type - generic_org_type_counts[generic_org_type] += 1 - else: - generic_org_types_query = DomainInformation.objects.filter(**filter_condition).values_list( - "generic_org_type", flat=True - ) - generic_org_type_counts = Counter(generic_org_types_query) - - # Extract counts for each generic_org_type - federal = generic_org_type_counts.get(DomainRequest.OrganizationChoices.FEDERAL, 0) - interstate = generic_org_type_counts.get(DomainRequest.OrganizationChoices.INTERSTATE, 0) - state_or_territory = generic_org_type_counts.get(DomainRequest.OrganizationChoices.STATE_OR_TERRITORY, 0) - tribal = generic_org_type_counts.get(DomainRequest.OrganizationChoices.TRIBAL, 0) - county = generic_org_type_counts.get(DomainRequest.OrganizationChoices.COUNTY, 0) - city = generic_org_type_counts.get(DomainRequest.OrganizationChoices.CITY, 0) - special_district = generic_org_type_counts.get(DomainRequest.OrganizationChoices.SPECIAL_DISTRICT, 0) - school_district = generic_org_type_counts.get(DomainRequest.OrganizationChoices.SCHOOL_DISTRICT, 0) - - # Round trip 3 - election_board = DomainInformation.objects.filter(is_election_board=True, **filter_condition).distinct().count() + 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() + state_or_territory = ( + domains.filter(generic_org_type=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() + special_district = ( + domains.filter(generic_org_type=DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + ) + school_district = ( + domains.filter(generic_org_type=DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + ) + election_board = domains.filter(is_election_board=True).distinct().count() return [ domains_count, @@ -588,26 +553,23 @@ def get_sliced_domains(filter_condition, distinct=False): def get_sliced_requests(filter_condition): """Get filtered requests counts sliced by org type and election office.""" - # Round trip 1: Get distinct requests based on filter condition - requests_count = DomainRequest.objects.filter(**filter_condition).distinct().count() - - # Round trip 2: Get counts for other slices - generic_org_types_query = DomainRequest.objects.filter(**filter_condition).values_list( - "generic_org_type", flat=True + 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() + state_or_territory = ( + requests.filter(generic_org_type=DomainRequest.OrganizationChoices.STATE_OR_TERRITORY).distinct().count() ) - generic_org_type_counts = Counter(generic_org_types_query) - - federal = generic_org_type_counts.get(DomainRequest.OrganizationChoices.FEDERAL, 0) - interstate = generic_org_type_counts.get(DomainRequest.OrganizationChoices.INTERSTATE, 0) - state_or_territory = generic_org_type_counts.get(DomainRequest.OrganizationChoices.STATE_OR_TERRITORY, 0) - tribal = generic_org_type_counts.get(DomainRequest.OrganizationChoices.TRIBAL, 0) - county = generic_org_type_counts.get(DomainRequest.OrganizationChoices.COUNTY, 0) - city = generic_org_type_counts.get(DomainRequest.OrganizationChoices.CITY, 0) - special_district = generic_org_type_counts.get(DomainRequest.OrganizationChoices.SPECIAL_DISTRICT, 0) - school_district = generic_org_type_counts.get(DomainRequest.OrganizationChoices.SCHOOL_DISTRICT, 0) - - # Round trip 3 - election_board = DomainRequest.objects.filter(is_election_board=True, **filter_condition).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() + special_district = ( + requests.filter(generic_org_type=DomainRequest.OrganizationChoices.SPECIAL_DISTRICT).distinct().count() + ) + school_district = ( + requests.filter(generic_org_type=DomainRequest.OrganizationChoices.SCHOOL_DISTRICT).distinct().count() + ) + election_board = requests.filter(is_election_board=True).distinct().count() return [ requests_count, diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index e33dea407..eba8423ed 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -41,29 +41,29 @@ class AnalyticsView(View): start_date_formatted = csv_export.format_start_date(start_date) end_date_formatted = csv_export.format_end_date(end_date) - # filter_managed_domains_start_date = { - # "domain__permissions__isnull": False, - # "domain__first_ready__lte": start_date_formatted, - # } - # filter_managed_domains_end_date = { - # "domain__permissions__isnull": False, - # "domain__first_ready__lte": end_date_formatted, - # } - # managed_domains_sliced_at_start_date = csv_export.get_sliced_domains(filter_managed_domains_start_date, True) - # managed_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_managed_domains_end_date, True) + filter_managed_domains_start_date = { + "domain__permissions__isnull": False, + "domain__first_ready__lte": start_date_formatted, + } + filter_managed_domains_end_date = { + "domain__permissions__isnull": False, + "domain__first_ready__lte": end_date_formatted, + } + managed_domains_sliced_at_start_date = csv_export.get_sliced_domains(filter_managed_domains_start_date, True) + managed_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_managed_domains_end_date, True) - # filter_unmanaged_domains_start_date = { - # "domain__permissions__isnull": True, - # "domain__first_ready__lte": start_date_formatted, - # } - # filter_unmanaged_domains_end_date = { - # "domain__permissions__isnull": True, - # "domain__first_ready__lte": end_date_formatted, - # } - # unmanaged_domains_sliced_at_start_date = csv_export.get_sliced_domains( - # filter_unmanaged_domains_start_date, True - # ) - # unmanaged_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_unmanaged_domains_end_date, True) + filter_unmanaged_domains_start_date = { + "domain__permissions__isnull": True, + "domain__first_ready__lte": start_date_formatted, + } + filter_unmanaged_domains_end_date = { + "domain__permissions__isnull": True, + "domain__first_ready__lte": end_date_formatted, + } + unmanaged_domains_sliced_at_start_date = csv_export.get_sliced_domains( + filter_unmanaged_domains_start_date, True + ) + unmanaged_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_unmanaged_domains_end_date, True) filter_ready_domains_start_date = { "domain__state__in": [models.Domain.State.READY], @@ -120,10 +120,10 @@ class AnalyticsView(View): last_30_days_applications=last_30_days_applications.count(), last_30_days_approved_applications=last_30_days_approved_applications.count(), average_application_approval_time_last_30_days=avg_approval_time_display, - # managed_domains_sliced_at_start_date=managed_domains_sliced_at_start_date, - # unmanaged_domains_sliced_at_start_date=unmanaged_domains_sliced_at_start_date, - # managed_domains_sliced_at_end_date=managed_domains_sliced_at_end_date, - # unmanaged_domains_sliced_at_end_date=unmanaged_domains_sliced_at_end_date, + managed_domains_sliced_at_start_date=managed_domains_sliced_at_start_date, + unmanaged_domains_sliced_at_start_date=unmanaged_domains_sliced_at_start_date, + managed_domains_sliced_at_end_date=managed_domains_sliced_at_end_date, + unmanaged_domains_sliced_at_end_date=unmanaged_domains_sliced_at_end_date, ready_domains_sliced_at_start_date=ready_domains_sliced_at_start_date, deleted_domains_sliced_at_start_date=deleted_domains_sliced_at_start_date, ready_domains_sliced_at_end_date=ready_domains_sliced_at_end_date, From 45c047dc8a5cf32a888d620dde33036b55b40df9 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 1 Apr 2024 21:35:29 -0400 Subject: [PATCH 06/13] optimize by removing filters from inside iterations --- src/registrar/utility/csv_export.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 0521a71be..d4485e11f 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -107,7 +107,7 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di # Get lists of emails for active and invited domain managers dm_active_emails = [dm.user.email for dm in domain.permissions.all()] dm_invited_emails = [ - invite.email for invite in invites_with_invited_status.filter(domain=domain) + invite.email for invite in invites_with_invited_status if invite.domain_id == domain_info.domain_id ] # Set up the "matching headers" + row field data for email and status @@ -160,7 +160,7 @@ def update_columns_with_domain_managers( Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" dm_active = domain_info.domain.permissions.count() - dm_invited = invites_with_invited_status.filter(domain=domain_info.domain).count() + dm_invited = sum(1 for invite in invites_with_invited_status if invite.domain_id == domain_info.domain_id) if dm_active + dm_invited > max_dm_total: max_dm_total = dm_active + dm_invited From dba2dfb6c2034668fc658f461262ba4f5a6c1c89 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 1 Apr 2024 21:46:06 -0400 Subject: [PATCH 07/13] Revert last experiment --- src/registrar/utility/csv_export.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d4485e11f..0521a71be 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -107,7 +107,7 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di # Get lists of emails for active and invited domain managers dm_active_emails = [dm.user.email for dm in domain.permissions.all()] dm_invited_emails = [ - invite.email for invite in invites_with_invited_status if invite.domain_id == domain_info.domain_id + invite.email for invite in invites_with_invited_status.filter(domain=domain) ] # Set up the "matching headers" + row field data for email and status @@ -160,7 +160,7 @@ def update_columns_with_domain_managers( Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" dm_active = domain_info.domain.permissions.count() - dm_invited = sum(1 for invite in invites_with_invited_status if invite.domain_id == domain_info.domain_id) + dm_invited = invites_with_invited_status.filter(domain=domain_info.domain).count() if dm_active + dm_invited > max_dm_total: max_dm_total = dm_active + dm_invited From 410440ee709b26e086f3c85b80c04a6ba1c5bb63 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 2 Apr 2024 12:39:29 -0400 Subject: [PATCH 08/13] Optimize by using prebuilt dicts --- src/registrar/utility/csv_export.py | 139 ++++++++++++++++++---------- 1 file changed, 88 insertions(+), 51 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 0521a71be..d8638d0d9 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -11,6 +11,8 @@ from django.db.models import F, Value, CharField, Q, Count from django.db.models.functions import Concat, Coalesce from registrar.models.public_contact import PublicContact +from registrar.models.user_domain_role import UserDomainRole +from registrar.models.utility.generic_helper import Timer from registrar.utility.enums import DefaultEmail logger = logging.getLogger(__name__) @@ -33,7 +35,6 @@ def get_domain_infos(filter_condition, sort_fields): """ domain_infos = ( DomainInformation.objects.select_related("domain", "authorizing_official") - .prefetch_related("domain__permissions", "domain__invitations") .filter(**filter_condition) .order_by(*sort_fields) .distinct() @@ -53,7 +54,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_domain_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False, invites_with_invited_status=None): +def parse_domain_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False, domain_invitation_emails=None, domain_permissions_emails=None): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -105,10 +106,9 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di if get_domain_managers: # Get lists of emails for active and invited domain managers - dm_active_emails = [dm.user.email for dm in domain.permissions.all()] - dm_invited_emails = [ - invite.email for invite in invites_with_invited_status.filter(domain=domain) - ] + + dm_active_emails = domain_permissions_emails.get(domain_info.domain.name, []) + dm_invited_emails = domain_invitation_emails.get(domain_info.domain.name, []) # Set up the "matching headers" + row field data for email and status i = 0 # Declare i outside of the loop to avoid a reference before assignment in the second loop @@ -148,7 +148,7 @@ def _get_security_emails(sec_contact_ids): def update_columns_with_domain_managers( - domain_info,invites_with_invited_status, update_columns, columns, max_dm_total + domain_info,domain_invitation_emails, domain_permissions_emails, update_columns, columns, max_dm_total ): """Helper function that works with 'global' variables set in write_domains_csv Accepts: @@ -159,8 +159,25 @@ def update_columns_with_domain_managers( Returns: Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" - dm_active = domain_info.domain.permissions.count() - dm_invited = invites_with_invited_status.filter(domain=domain_info.domain).count() + dm_active = 0 + dm_invited = 0 + try: + # logger.info(f'domain_invitation_emails {domain_invitation_emails[domain_info.domain.name]}') + + # Get the list of invitation emails for the domain name if it exists, otherwise, return an empty list + invitation_emails = domain_invitation_emails.get(domain_info.domain.name, []) + # Count the number of invitation emails + dm_invited = len(invitation_emails) + except KeyError: + pass + + try: + active_emails = domain_permissions_emails.get(domain_info.domain.name, []) + # Count the number of invitation emails + dm_active = len(active_emails) + except KeyError: + pass + if dm_active + dm_invited > max_dm_total: max_dm_total = dm_active + dm_invited @@ -193,61 +210,80 @@ def write_domains_csv( should_write_header: Conditional bc export_data_domain_growth_to_csv calls write_body twice """ - all_domain_infos = get_domain_infos(filter_condition, sort_fields) + with Timer(): + all_domain_infos = get_domain_infos(filter_condition, sort_fields) - # Store all security emails to avoid epp calls or excessive filters - sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) + # Store all security emails to avoid epp calls or excessive filters + sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) - security_emails_dict = _get_security_emails(sec_contact_ids) + security_emails_dict = _get_security_emails(sec_contact_ids) - # Reduce the memory overhead when performing the write operation - paginator = Paginator(all_domain_infos, 1000) + # Reduce the memory overhead when performing the write operation + paginator = Paginator(all_domain_infos, 1000) - # We get the number of domain managers (DMs) an the domain - # that has the most DMs so we can set the header row appropriately + # We get the number of domain managers (DMs) an the domain + # that has the most DMs so we can set the header row appropriately - max_dm_total = 0 - update_columns = False - invites_with_invited_status=None + max_dm_total = 0 + update_columns = False + + invites_with_invited_status=None + domain_invitation_emails = {} + domain_permissions_emails = {} - if get_domain_managers: - invites_with_invited_status = DomainInvitation.objects.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).prefetch_related("domain") + if get_domain_managers: + invites_with_invited_status = DomainInvitation.objects.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).select_related("domain") - # zander = DomainInformation.objects.filter(**filter_condition).annotate(invitations_count=Count('invitation', filter=Q(invitation__status='invited'))).values_list('domain_name', 'invitations_count') - # logger.info(f'zander {zander}') - # zander_dict = dict(zander) - # logger.info(f'zander_dict {zander_dict}') + # Iterate through each domain invitation and populate the dictionary + for invite in invites_with_invited_status: + domain_name = invite.domain.name + email = invite.email + if domain_name not in domain_invitation_emails: + domain_invitation_emails[domain_name] = [] + domain_invitation_emails[domain_name].append(email) - # This var will live outside of the nested for loops to aggregate - # the data from those loops - total_body_rows = [] + domain_permissions = UserDomainRole.objects.all() - for page_num in paginator.page_range: - rows = [] - page = paginator.page(page_num) - for domain_info in page.object_list: + # Iterate through each domain invitation and populate the dictionary + for permission in domain_permissions: + domain_name = permission.domain.name + email = permission.user.email + if domain_name not in domain_permissions_emails: + domain_permissions_emails[domain_name] = [] + domain_permissions_emails[domain_name].append(email) - # Get max number of domain managers - if get_domain_managers: - update_columns, columns, max_dm_total = ( - update_columns_with_domain_managers( - domain_info,invites_with_invited_status, update_columns, columns, max_dm_total + # logger.info(f'domain_invitation_emails {domain_invitation_emails}') + + # This var will live outside of the nested for loops to aggregate + # the data from those loops + total_body_rows = [] + + for page_num in paginator.page_range: + rows = [] + page = paginator.page(page_num) + for domain_info in page.object_list: + + # Get max number of domain managers + if get_domain_managers: + update_columns, columns, max_dm_total = ( + update_columns_with_domain_managers( + domain_info, domain_invitation_emails,domain_permissions_emails, update_columns, columns, max_dm_total + ) ) - ) - try: - row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers, invites_with_invited_status) - rows.append(row) - except ValueError: - # This should not happen. If it does, just skip this row. - # It indicates that DomainInformation.domain is None. - logger.error("csv_export -> Error when parsing row, domain was None") - continue - total_body_rows.extend(rows) + try: + row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers, domain_invitation_emails,domain_permissions_emails) + rows.append(row) + except ValueError: + # This should not happen. If it does, just skip this row. + # It indicates that DomainInformation.domain is None. + logger.error("csv_export -> Error when parsing row, domain was None") + continue + total_body_rows.extend(rows) - if should_write_header: - write_header(writer, columns) - writer.writerows(total_body_rows) + if should_write_header: + write_header(writer, columns) + writer.writerows(total_body_rows) def get_requests(filter_condition, sort_fields): @@ -360,6 +396,7 @@ def export_data_type_to_csv(csv_file): Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, + Domain.State.UNKNOWN, ], } write_domains_csv( From 045140091b11254b389eb0329fb70b342d2cedf3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 2 Apr 2024 12:44:00 -0400 Subject: [PATCH 09/13] do not pull unknown domains --- src/registrar/utility/csv_export.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d8638d0d9..913fe95db 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -396,7 +396,6 @@ def export_data_type_to_csv(csv_file): Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, - Domain.State.UNKNOWN, ], } write_domains_csv( From c5e6295a8a5c25f480c0465b2dd328224463cde1 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 2 Apr 2024 20:54:43 -0400 Subject: [PATCH 10/13] refactor wip --- src/registrar/tests/test_reports.py | 39 +++-- src/registrar/utility/csv_export.py | 247 +++++++++++++++------------- src/registrar/views/admin_views.py | 10 +- 3 files changed, 162 insertions(+), 134 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index cd882c4f8..b4861560f 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -243,7 +243,12 @@ class ExportDataTest(MockDb, MockEppLib): self.maxDiff = None # Call the export functions write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=False, + should_write_header=True, ) # Reset the CSV file's position to the beginning @@ -305,7 +310,12 @@ class ExportDataTest(MockDb, MockEppLib): } # Call the export functions write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=False, + should_write_header=True, ) # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -358,7 +368,12 @@ class ExportDataTest(MockDb, MockEppLib): } # Call the export functions write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=False, + should_write_header=True, ) # Reset the CSV file's position to the beginning csv_file.seek(0) @@ -438,7 +453,7 @@ class ExportDataTest(MockDb, MockEppLib): columns, sort_fields, filter_condition, - get_domain_managers=False, + should_get_domain_managers=False, should_write_header=True, ) write_domains_csv( @@ -446,7 +461,7 @@ class ExportDataTest(MockDb, MockEppLib): columns, sort_fields_for_deleted_domains, filter_conditions_for_deleted_domains, - get_domain_managers=False, + should_get_domain_managers=False, should_write_header=False, ) # Reset the CSV file's position to the beginning @@ -514,7 +529,12 @@ class ExportDataTest(MockDb, MockEppLib): self.maxDiff = None # Call the export functions write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True + writer, + columns, + sort_fields, + filter_condition, + should_get_domain_managers=True, + should_write_header=True, ) # Reset the CSV file's position to the beginning @@ -697,13 +717,8 @@ class HelperFunctions(MockDb): "domain__first_ready__lte": self.end_date, } # Test with distinct - managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition, True) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 2] - self.assertEqual(managed_domains_sliced_at_end_date, expected_content) - - # Test without distinct managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 4, 1, 0, 0, 0, 0, 0, 0, 2] + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 2] self.assertEqual(managed_domains_sliced_at_end_date, expected_content) def test_get_sliced_requests(self): diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 913fe95db..faad25b28 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -7,7 +7,7 @@ from registrar.models.domain_request import DomainRequest from registrar.models.domain_information import DomainInformation from django.utils import timezone from django.core.paginator import Paginator -from django.db.models import F, Value, CharField, Q, Count +from django.db.models import F, Value, CharField from django.db.models.functions import Concat, Coalesce from registrar.models.public_contact import PublicContact @@ -54,7 +54,14 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_domain_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False, domain_invitation_emails=None, domain_permissions_emails=None): +def parse_domain_row( + columns, + domain_info: DomainInformation, + dict_security_emails_dict=None, + should_get_domain_managers=False, + dict_domain_invitations_with_invited_status=None, + dict_user_domain_roles=None, +): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -66,8 +73,8 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di # Grab the security email from a preset dictionary. # If nothing exists in the dictionary, grab from .contacts. - if security_emails_dict is not None and domain.name in security_emails_dict: - _email = security_emails_dict.get(domain.name) + if dict_security_emails_dict is not None and domain.name in dict_security_emails_dict: + _email = dict_security_emails_dict.get(domain.name) security_email = _email if _email is not None else " " else: # If the dictionary doesn't contain that data, lets filter for it manually. @@ -104,20 +111,20 @@ def parse_domain_row(columns, domain_info: DomainInformation, security_emails_di "Deleted": domain.deleted, } - if get_domain_managers: + if should_get_domain_managers: # Get lists of emails for active and invited domain managers - dm_active_emails = domain_permissions_emails.get(domain_info.domain.name, []) - dm_invited_emails = domain_invitation_emails.get(domain_info.domain.name, []) + dms_active_emails = dict_user_domain_roles.get(domain_info.domain.name, []) + dms_invited_emails = dict_domain_invitations_with_invited_status.get(domain_info.domain.name, []) # Set up the "matching headers" + row field data for email and status i = 0 # Declare i outside of the loop to avoid a reference before assignment in the second loop - for i, dm_email in enumerate(dm_active_emails, start=1): + for i, dm_email in enumerate(dms_active_emails, start=1): FIELDS[f"Domain manager {i}"] = dm_email FIELDS[f"DM{i} status"] = "R" # Continue enumeration from where we left off and add data for invited domain managers - for j, dm_email in enumerate(dm_invited_emails, start=i + 1): + for j, dm_email in enumerate(dms_invited_emails, start=i + 1): FIELDS[f"Domain manager {j}"] = dm_email FIELDS[f"DM{j} status"] = "I" @@ -129,7 +136,7 @@ def _get_security_emails(sec_contact_ids): """ Retrieve security contact emails for the given security contact IDs. """ - security_emails_dict = {} + dict_security_emails_dict = {} public_contacts = ( PublicContact.objects.only("email", "domain__name") .select_related("domain") @@ -139,144 +146,152 @@ def _get_security_emails(sec_contact_ids): # Populate a dictionary of domain names and their security contacts for contact in public_contacts: domain: Domain = contact.domain - if domain is not None and domain.name not in security_emails_dict: - security_emails_dict[domain.name] = contact.email + if domain is not None and domain.name not in dict_security_emails_dict: + dict_security_emails_dict[domain.name] = contact.email else: logger.warning("csv_export -> Domain was none for PublicContact") - return security_emails_dict + return dict_security_emails_dict + + +def count_domain_managers(domain_name, dict_domain_invitations_with_invited_status, dict_user_domain_roles): + """Count active and invited domain managers""" + dms_active = len(dict_user_domain_roles.get(domain_name, [])) + dms_invited = len(dict_domain_invitations_with_invited_status.get(domain_name, [])) + return dms_active, dms_invited + + +def update_columns(columns, dms_total, should_update_columns): + """Update columns if necessary""" + if should_update_columns: + for i in range(1, dms_total + 1): + email_column_header = f"Domain manager {i}" + status_column_header = f"DM{i} status" + if email_column_header not in columns: + columns.append(email_column_header) + columns.append(status_column_header) + should_update_columns = False + return columns, should_update_columns, dms_total def update_columns_with_domain_managers( - domain_info,domain_invitation_emails, domain_permissions_emails, update_columns, columns, max_dm_total + columns, + domain_info, + should_update_columns, + dms_total, + dict_domain_invitations_with_invited_status, + dict_user_domain_roles, ): - """Helper function that works with 'global' variables set in write_domains_csv - Accepts: - domain_info -> Domains to parse - update_columns -> A control to make sure we only run the columns test and update when needed - columns -> The header cells in the csv that's under construction - max_dm_total -> Starts at 0 and gets updated and passed again through this method - Returns: - Updated update_columns, columns, max_dm_active, max_dm_invited, max_dm_total""" + """Helper function to update columns with domain manager information""" - dm_active = 0 - dm_invited = 0 - try: - # logger.info(f'domain_invitation_emails {domain_invitation_emails[domain_info.domain.name]}') - - # Get the list of invitation emails for the domain name if it exists, otherwise, return an empty list - invitation_emails = domain_invitation_emails.get(domain_info.domain.name, []) - # Count the number of invitation emails - dm_invited = len(invitation_emails) - except KeyError: - pass + domain_name = domain_info.domain.name try: - active_emails = domain_permissions_emails.get(domain_info.domain.name, []) - # Count the number of invitation emails - dm_active = len(active_emails) - except KeyError: - pass - + dms_active, dms_invited = count_domain_managers( + domain_name, dict_domain_invitations_with_invited_status, dict_user_domain_roles + ) - if dm_active + dm_invited > max_dm_total: - max_dm_total = dm_active + dm_invited - update_columns = True + if dms_active + dms_invited > dms_total: + dms_total = dms_active + dms_invited + should_update_columns = True - if update_columns: - for i in range(1, max_dm_total + 1): - column_name = f"Domain manager {i}" - column2_name = f"DM{i} status" - if column_name not in columns: - columns.append(column_name) - columns.append(column2_name) - update_columns = False + except Exception as err: + logger.error(f"Exception while parsing domain managers for reports: {err}") - return update_columns, columns, max_dm_total + return update_columns(columns, dms_total, should_update_columns) +def build_dictionaries_for_domain_managers(dict_user_domain_roles, dict_domain_invitations_with_invited_status): + """Helper function that builds dicts for invited users and active domain + managers. We do so to avoid filtering within loops.""" + + user_domain_roles = None + user_domain_roles = UserDomainRole.objects.all() + + # Iterate through each user domain role and populate the dictionary + for user_domain_role in user_domain_roles: + domain_name = user_domain_role.domain.name + email = user_domain_role.user.email + if domain_name not in dict_user_domain_roles: + dict_user_domain_roles[domain_name] = [] + dict_user_domain_roles[domain_name].append(email) + + domain_invitations_with_invited_status = None + domain_invitations_with_invited_status = DomainInvitation.objects.filter( + status=DomainInvitation.DomainInvitationStatus.INVITED + ).select_related("domain") + + # Iterate through each domain invitation and populate the dictionary + for invite in domain_invitations_with_invited_status: + domain_name = invite.domain.name + email = invite.email + if domain_name not in dict_domain_invitations_with_invited_status: + dict_domain_invitations_with_invited_status[domain_name] = [] + dict_domain_invitations_with_invited_status[domain_name].append(email) + + return dict_user_domain_roles, dict_domain_invitations_with_invited_status + def write_domains_csv( writer, columns, sort_fields, filter_condition, - get_domain_managers=False, + should_get_domain_managers=False, should_write_header=True, ): """ Receives params from the parent methods and outputs a CSV with filtered and sorted domains. Works with write_header as long as the same writer object is passed. - get_domain_managers: Conditional bc we only use domain manager info for export_data_full_to_csv + should_get_domain_managers: Conditional bc we only use domain manager info for export_data_full_to_csv should_write_header: Conditional bc export_data_domain_growth_to_csv calls write_body twice """ with Timer(): + # Retrieve domain information and all sec emails all_domain_infos = get_domain_infos(filter_condition, sort_fields) - - # Store all security emails to avoid epp calls or excessive filters sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) - - security_emails_dict = _get_security_emails(sec_contact_ids) - - # Reduce the memory overhead when performing the write operation + dict_security_emails_dict = _get_security_emails(sec_contact_ids) paginator = Paginator(all_domain_infos, 1000) - # We get the number of domain managers (DMs) an the domain - # that has the most DMs so we can set the header row appropriately - - max_dm_total = 0 - update_columns = False - - invites_with_invited_status=None - domain_invitation_emails = {} - domain_permissions_emails = {} - - if get_domain_managers: - invites_with_invited_status = DomainInvitation.objects.filter(status=DomainInvitation.DomainInvitationStatus.INVITED).select_related("domain") - - # Iterate through each domain invitation and populate the dictionary - for invite in invites_with_invited_status: - domain_name = invite.domain.name - email = invite.email - if domain_name not in domain_invitation_emails: - domain_invitation_emails[domain_name] = [] - domain_invitation_emails[domain_name].append(email) - - domain_permissions = UserDomainRole.objects.all() - - # Iterate through each domain invitation and populate the dictionary - for permission in domain_permissions: - domain_name = permission.domain.name - email = permission.user.email - if domain_name not in domain_permissions_emails: - domain_permissions_emails[domain_name] = [] - domain_permissions_emails[domain_name].append(email) - - # logger.info(f'domain_invitation_emails {domain_invitation_emails}') - - # This var will live outside of the nested for loops to aggregate - # the data from those loops + # Initialize variables + dms_total = 0 + should_update_columns = False total_body_rows = [] + dict_user_domain_roles = {} + dict_domain_invitations_with_invited_status = {} + # Build dictionaries if necessary + if should_get_domain_managers: + dict_user_domain_roles, dict_domain_invitations_with_invited_status = build_dictionaries_for_domain_managers( + dict_user_domain_roles, dict_domain_invitations_with_invited_status + ) + + # Process domain information for page_num in paginator.page_range: rows = [] page = paginator.page(page_num) for domain_info in page.object_list: - - # Get max number of domain managers - if get_domain_managers: - update_columns, columns, max_dm_total = ( - update_columns_with_domain_managers( - domain_info, domain_invitation_emails,domain_permissions_emails, update_columns, columns, max_dm_total - ) + if should_get_domain_managers: + columns, dms_total, should_update_columns = update_columns_with_domain_managers( + columns, + domain_info, + should_update_columns, + dms_total, + dict_domain_invitations_with_invited_status, + dict_user_domain_roles, ) try: - row = parse_domain_row(columns, domain_info, security_emails_dict, get_domain_managers, domain_invitation_emails,domain_permissions_emails) + row = parse_domain_row( + columns, + domain_info, + dict_security_emails_dict, + should_get_domain_managers, + dict_domain_invitations_with_invited_status, + dict_user_domain_roles, + ) rows.append(row) except ValueError: - # This should not happen. If it does, just skip this row. - # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue total_body_rows.extend(rows) @@ -399,7 +414,7 @@ def export_data_type_to_csv(csv_file): ], } write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=True, should_write_header=True + writer, columns, sort_fields, filter_condition, should_get_domain_managers=True, should_write_header=True ) @@ -432,7 +447,7 @@ def export_data_full_to_csv(csv_file): ], } write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) @@ -466,7 +481,7 @@ def export_data_federal_to_csv(csv_file): ], } write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) @@ -536,19 +551,19 @@ def export_data_domain_growth_to_csv(csv_file, start_date, end_date): } write_domains_csv( - writer, columns, sort_fields, filter_condition, get_domain_managers=False, should_write_header=True + writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) write_domains_csv( writer, columns, sort_fields_for_deleted_domains, filter_condition_for_deleted_domains, - get_domain_managers=False, + should_get_domain_managers=False, should_write_header=False, ) -def get_sliced_domains(filter_condition, distinct=False): +def get_sliced_domains(filter_condition): """Get filtered domains counts sliced by org type and election office. Pass distinct=True when filtering by permissions so we do not to count multiples when a domain has more that one manager. @@ -639,7 +654,7 @@ def export_data_managed_domains_to_csv(csv_file, start_date, end_date): "domain__permissions__isnull": False, "domain__first_ready__lte": start_date_formatted, } - managed_domains_sliced_at_start_date = get_sliced_domains(filter_managed_domains_start_date, True) + managed_domains_sliced_at_start_date = get_sliced_domains(filter_managed_domains_start_date) writer.writerow(["MANAGED DOMAINS COUNTS AT START DATE"]) writer.writerow( @@ -663,7 +678,7 @@ def export_data_managed_domains_to_csv(csv_file, start_date, end_date): "domain__permissions__isnull": False, "domain__first_ready__lte": end_date_formatted, } - managed_domains_sliced_at_end_date = get_sliced_domains(filter_managed_domains_end_date, True) + managed_domains_sliced_at_end_date = get_sliced_domains(filter_managed_domains_end_date) writer.writerow(["MANAGED DOMAINS COUNTS AT END DATE"]) writer.writerow( @@ -688,7 +703,7 @@ def export_data_managed_domains_to_csv(csv_file, start_date, end_date): columns, sort_fields, filter_managed_domains_end_date, - get_domain_managers=True, + should_get_domain_managers=True, should_write_header=True, ) @@ -712,7 +727,7 @@ def export_data_unmanaged_domains_to_csv(csv_file, start_date, end_date): "domain__permissions__isnull": True, "domain__first_ready__lte": start_date_formatted, } - unmanaged_domains_sliced_at_start_date = get_sliced_domains(filter_unmanaged_domains_start_date, True) + unmanaged_domains_sliced_at_start_date = get_sliced_domains(filter_unmanaged_domains_start_date) writer.writerow(["UNMANAGED DOMAINS AT START DATE"]) writer.writerow( @@ -736,7 +751,7 @@ def export_data_unmanaged_domains_to_csv(csv_file, start_date, end_date): "domain__permissions__isnull": True, "domain__first_ready__lte": end_date_formatted, } - unmanaged_domains_sliced_at_end_date = get_sliced_domains(filter_unmanaged_domains_end_date, True) + unmanaged_domains_sliced_at_end_date = get_sliced_domains(filter_unmanaged_domains_end_date) writer.writerow(["UNMANAGED DOMAINS AT END DATE"]) writer.writerow( @@ -761,7 +776,7 @@ def export_data_unmanaged_domains_to_csv(csv_file, start_date, end_date): columns, sort_fields, filter_unmanaged_domains_end_date, - get_domain_managers=False, + should_get_domain_managers=False, should_write_header=True, ) diff --git a/src/registrar/views/admin_views.py b/src/registrar/views/admin_views.py index eba8423ed..01a8157f9 100644 --- a/src/registrar/views/admin_views.py +++ b/src/registrar/views/admin_views.py @@ -49,8 +49,8 @@ class AnalyticsView(View): "domain__permissions__isnull": False, "domain__first_ready__lte": end_date_formatted, } - managed_domains_sliced_at_start_date = csv_export.get_sliced_domains(filter_managed_domains_start_date, True) - managed_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_managed_domains_end_date, True) + managed_domains_sliced_at_start_date = csv_export.get_sliced_domains(filter_managed_domains_start_date) + managed_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_managed_domains_end_date) filter_unmanaged_domains_start_date = { "domain__permissions__isnull": True, @@ -60,10 +60,8 @@ class AnalyticsView(View): "domain__permissions__isnull": True, "domain__first_ready__lte": end_date_formatted, } - unmanaged_domains_sliced_at_start_date = csv_export.get_sliced_domains( - filter_unmanaged_domains_start_date, True - ) - unmanaged_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_unmanaged_domains_end_date, True) + unmanaged_domains_sliced_at_start_date = csv_export.get_sliced_domains(filter_unmanaged_domains_start_date) + unmanaged_domains_sliced_at_end_date = csv_export.get_sliced_domains(filter_unmanaged_domains_end_date) filter_ready_domains_start_date = { "domain__state__in": [models.Domain.State.READY], From 77d1158c1b34b8c40bd139009bb5c7784fad4149 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 2 Apr 2024 21:49:42 -0400 Subject: [PATCH 11/13] Clean up --- src/registrar/tests/test_reports.py | 20 ++--- src/registrar/utility/csv_export.py | 123 ++++++++++++++-------------- 2 files changed, 71 insertions(+), 72 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index b4861560f..b34f3d920 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -9,10 +9,10 @@ from registrar.utility.csv_export import ( export_data_unmanaged_domains_to_csv, get_sliced_domains, get_sliced_requests, - write_domains_csv, + write_csv_for_domains, get_default_start_date, get_default_end_date, - write_requests_csv, + write_csv_for_requests, ) from django.core.management import call_command @@ -242,7 +242,7 @@ class ExportDataTest(MockDb, MockEppLib): } self.maxDiff = None # Call the export functions - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -273,7 +273,7 @@ class ExportDataTest(MockDb, MockEppLib): expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() self.assertEqual(csv_content, expected_content) - def test_write_domains_csv(self): + def test_write_csv_for_domains(self): """Test that write_body returns the existing domain, test that sort by domain name works, test that filter works""" @@ -309,7 +309,7 @@ class ExportDataTest(MockDb, MockEppLib): ], } # Call the export functions - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -367,7 +367,7 @@ class ExportDataTest(MockDb, MockEppLib): ], } # Call the export functions - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -448,7 +448,7 @@ class ExportDataTest(MockDb, MockEppLib): } # Call the export functions - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -456,7 +456,7 @@ class ExportDataTest(MockDb, MockEppLib): should_get_domain_managers=False, should_write_header=True, ) - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields_for_deleted_domains, @@ -528,7 +528,7 @@ class ExportDataTest(MockDb, MockEppLib): } self.maxDiff = None # Call the export functions - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -673,7 +673,7 @@ class ExportDataTest(MockDb, MockEppLib): "submission_date__lte": self.end_date, "submission_date__gte": self.start_date, } - write_requests_csv(writer, columns, sort_fields, filter_condition, should_write_header=True) + write_csv_for_requests(writer, columns, sort_fields, filter_condition, should_write_header=True) # Reset the CSV file's position to the beginning csv_file.seek(0) # Read the content into a variable diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index faad25b28..7605d5bfe 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -12,7 +12,6 @@ from django.db.models.functions import Concat, Coalesce from registrar.models.public_contact import PublicContact from registrar.models.user_domain_role import UserDomainRole -from registrar.models.utility.generic_helper import Timer from registrar.utility.enums import DefaultEmail logger = logging.getLogger(__name__) @@ -54,7 +53,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_domain_row( +def parse_row_for_domain( columns, domain_info: DomainInformation, dict_security_emails_dict=None, @@ -231,7 +230,8 @@ def build_dictionaries_for_domain_managers(dict_user_domain_roles, dict_domain_i return dict_user_domain_roles, dict_domain_invitations_with_invited_status -def write_domains_csv( + +def write_csv_for_domains( writer, columns, sort_fields, @@ -246,59 +246,58 @@ def write_domains_csv( should_write_header: Conditional bc export_data_domain_growth_to_csv calls write_body twice """ - with Timer(): - # Retrieve domain information and all sec emails - all_domain_infos = get_domain_infos(filter_condition, sort_fields) - sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) - dict_security_emails_dict = _get_security_emails(sec_contact_ids) - paginator = Paginator(all_domain_infos, 1000) + # Retrieve domain information and all sec emails + all_domain_infos = get_domain_infos(filter_condition, sort_fields) + sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) + dict_security_emails_dict = _get_security_emails(sec_contact_ids) + paginator = Paginator(all_domain_infos, 1000) - # Initialize variables - dms_total = 0 - should_update_columns = False - total_body_rows = [] - dict_user_domain_roles = {} - dict_domain_invitations_with_invited_status = {} + # Initialize variables + dms_total = 0 + should_update_columns = False + total_body_rows = [] + dict_user_domain_roles = {} + dict_domain_invitations_with_invited_status = {} - # Build dictionaries if necessary - if should_get_domain_managers: - dict_user_domain_roles, dict_domain_invitations_with_invited_status = build_dictionaries_for_domain_managers( - dict_user_domain_roles, dict_domain_invitations_with_invited_status - ) + # Build dictionaries if necessary + if should_get_domain_managers: + dict_user_domain_roles, dict_domain_invitations_with_invited_status = build_dictionaries_for_domain_managers( + dict_user_domain_roles, dict_domain_invitations_with_invited_status + ) - # Process domain information - for page_num in paginator.page_range: - rows = [] - page = paginator.page(page_num) - for domain_info in page.object_list: - if should_get_domain_managers: - columns, dms_total, should_update_columns = update_columns_with_domain_managers( - columns, - domain_info, - should_update_columns, - dms_total, - dict_domain_invitations_with_invited_status, - dict_user_domain_roles, - ) + # Process domain information + for page_num in paginator.page_range: + rows = [] + page = paginator.page(page_num) + for domain_info in page.object_list: + if should_get_domain_managers: + columns, dms_total, should_update_columns = update_columns_with_domain_managers( + columns, + domain_info, + should_update_columns, + dms_total, + dict_domain_invitations_with_invited_status, + dict_user_domain_roles, + ) - try: - row = parse_domain_row( - columns, - domain_info, - dict_security_emails_dict, - should_get_domain_managers, - dict_domain_invitations_with_invited_status, - dict_user_domain_roles, - ) - rows.append(row) - except ValueError: - logger.error("csv_export -> Error when parsing row, domain was None") - continue - total_body_rows.extend(rows) + try: + row = parse_row_for_domain( + columns, + domain_info, + dict_security_emails_dict, + should_get_domain_managers, + dict_domain_invitations_with_invited_status, + dict_user_domain_roles, + ) + rows.append(row) + except ValueError: + logger.error("csv_export -> Error when parsing row, domain was None") + continue + total_body_rows.extend(rows) - if should_write_header: - write_header(writer, columns) - writer.writerows(total_body_rows) + if should_write_header: + write_header(writer, columns) + writer.writerows(total_body_rows) def get_requests(filter_condition, sort_fields): @@ -312,7 +311,7 @@ def get_requests(filter_condition, sort_fields): return requests -def parse_request_row(columns, request: DomainRequest): +def parse_row_for_requests(columns, request: DomainRequest): """Given a set of columns, generate a new row from cleaned column data""" requested_domain_name = "No requested domain" @@ -344,7 +343,7 @@ def parse_request_row(columns, request: DomainRequest): return row -def write_requests_csv( +def write_csv_for_requests( writer, columns, sort_fields, @@ -365,7 +364,7 @@ def write_requests_csv( rows = [] for request in page.object_list: try: - row = parse_request_row(columns, request) + row = parse_row_for_requests(columns, request) rows.append(row) except ValueError: # This should not happen. If it does, just skip this row. @@ -413,7 +412,7 @@ def export_data_type_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, filter_condition, should_get_domain_managers=True, should_write_header=True ) @@ -446,7 +445,7 @@ def export_data_full_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) @@ -480,7 +479,7 @@ def export_data_federal_to_csv(csv_file): Domain.State.ON_HOLD, ], } - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) @@ -550,10 +549,10 @@ def export_data_domain_growth_to_csv(csv_file, start_date, end_date): "domain__deleted__gte": start_date_formatted, } - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, filter_condition, should_get_domain_managers=False, should_write_header=True ) - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields_for_deleted_domains, @@ -698,7 +697,7 @@ def export_data_managed_domains_to_csv(csv_file, start_date, end_date): writer.writerow(managed_domains_sliced_at_end_date) writer.writerow([]) - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -771,7 +770,7 @@ def export_data_unmanaged_domains_to_csv(csv_file, start_date, end_date): writer.writerow(unmanaged_domains_sliced_at_end_date) writer.writerow([]) - write_domains_csv( + write_csv_for_domains( writer, columns, sort_fields, @@ -807,4 +806,4 @@ def export_data_requests_growth_to_csv(csv_file, start_date, end_date): "submission_date__gte": start_date_formatted, } - write_requests_csv(writer, columns, sort_fields, filter_condition, should_write_header=True) + write_csv_for_requests(writer, columns, sort_fields, filter_condition, should_write_header=True) From 9d8d90e45a1f6bf418c9c35cac268601f5b130cf Mon Sep 17 00:00:00 2001 From: Rachid Mrad <107004823+rachidatecs@users.noreply.github.com> Date: Wed, 3 Apr 2024 11:26:23 -0400 Subject: [PATCH 12/13] Update src/registrar/utility/csv_export.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/utility/csv_export.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 7605d5bfe..01eef295a 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -204,7 +204,6 @@ def build_dictionaries_for_domain_managers(dict_user_domain_roles, dict_domain_i """Helper function that builds dicts for invited users and active domain managers. We do so to avoid filtering within loops.""" - user_domain_roles = None user_domain_roles = UserDomainRole.objects.all() # Iterate through each user domain role and populate the dictionary From 602f07f2194629a3fa2130d1d2c5b7224587eb26 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 3 Apr 2024 11:31:46 -0400 Subject: [PATCH 13/13] var name correction --- src/registrar/utility/csv_export.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 01eef295a..949b0adcd 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -56,7 +56,7 @@ def get_domain_infos(filter_condition, sort_fields): def parse_row_for_domain( columns, domain_info: DomainInformation, - dict_security_emails_dict=None, + dict_security_emails=None, should_get_domain_managers=False, dict_domain_invitations_with_invited_status=None, dict_user_domain_roles=None, @@ -72,8 +72,8 @@ def parse_row_for_domain( # Grab the security email from a preset dictionary. # If nothing exists in the dictionary, grab from .contacts. - if dict_security_emails_dict is not None and domain.name in dict_security_emails_dict: - _email = dict_security_emails_dict.get(domain.name) + if dict_security_emails is not None and domain.name in dict_security_emails: + _email = dict_security_emails.get(domain.name) security_email = _email if _email is not None else " " else: # If the dictionary doesn't contain that data, lets filter for it manually. @@ -135,7 +135,7 @@ def _get_security_emails(sec_contact_ids): """ Retrieve security contact emails for the given security contact IDs. """ - dict_security_emails_dict = {} + dict_security_emails = {} public_contacts = ( PublicContact.objects.only("email", "domain__name") .select_related("domain") @@ -145,12 +145,12 @@ def _get_security_emails(sec_contact_ids): # Populate a dictionary of domain names and their security contacts for contact in public_contacts: domain: Domain = contact.domain - if domain is not None and domain.name not in dict_security_emails_dict: - dict_security_emails_dict[domain.name] = contact.email + if domain is not None and domain.name not in dict_security_emails: + dict_security_emails[domain.name] = contact.email else: logger.warning("csv_export -> Domain was none for PublicContact") - return dict_security_emails_dict + return dict_security_emails def count_domain_managers(domain_name, dict_domain_invitations_with_invited_status, dict_user_domain_roles): @@ -248,7 +248,7 @@ def write_csv_for_domains( # Retrieve domain information and all sec emails all_domain_infos = get_domain_infos(filter_condition, sort_fields) sec_contact_ids = all_domain_infos.values_list("domain__security_contact_registry_id", flat=True) - dict_security_emails_dict = _get_security_emails(sec_contact_ids) + dict_security_emails = _get_security_emails(sec_contact_ids) paginator = Paginator(all_domain_infos, 1000) # Initialize variables @@ -283,7 +283,7 @@ def write_csv_for_domains( row = parse_row_for_domain( columns, domain_info, - dict_security_emails_dict, + dict_security_emails, should_get_domain_managers, dict_domain_invitations_with_invited_status, dict_user_domain_roles,