From 7e271fc273d0abc300073fc0e7bd5425a58dabd3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 20:50:50 -0500 Subject: [PATCH 1/9] refactor max_dm_count --- src/registrar/utility/csv_export.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 04a11c924..23a3808f8 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -25,7 +25,7 @@ def write_header(writer, columns): def get_domain_infos(filter_condition, sort_fields): domain_infos = ( - DomainInformation.objects.select_related("domain", "authorizing_official") + DomainInformation.objects.select_related("domain", "domain__permissions", "authorizing_official") .filter(**filter_condition) .order_by(*sort_fields) ) @@ -161,19 +161,20 @@ def write_csv( # Reduce the memory overhead when performing the write operation paginator = Paginator(all_domain_infos, 1000) - if get_domain_managers and len(all_domain_infos) > 0: - # We want to get the max amont of domain managers an - # account has to set the column header dynamically - max_dm_count = max(len(domain_info.domain.permissions.all()) for domain_info in all_domain_infos) - update_columns_with_domain_managers(columns, max_dm_count) - - if should_write_header: - write_header(writer, columns) + # The maximum amount of domain managers an account has + # We get the max so we can set the column header accurately + max_dm_count = 0 + total_body_rows = [] for page_num in paginator.page_range: rows = [] page = paginator.page(page_num) for domain_info in page.object_list: + # Get count of all the domain managers for an account + dm_count = len(domain_info.domain.permissions) + if dm_count > max_dm_count: + max_dm_count = dm_count + try: row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) @@ -182,8 +183,11 @@ def write_csv( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue + total_body_rows.append(rows) - writer.writerows(rows) + update_columns_with_domain_managers(columns, max_dm_count) + write_header(writer, columns) + writer.writerows(total_body_rows) def export_data_type_to_csv(csv_file): From dec1d4b5ea9960d7e3125c0fe0d7639cd1134479 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 21:00:20 -0500 Subject: [PATCH 2/9] fix prefecth domain__permissions --- src/registrar/utility/csv_export.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 23a3808f8..d325b95ea 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -25,7 +25,8 @@ def write_header(writer, columns): def get_domain_infos(filter_condition, sort_fields): domain_infos = ( - DomainInformation.objects.select_related("domain", "domain__permissions", "authorizing_official") + DomainInformation.objects.select_related("domain", "authorizing_official") + .prefetch_related("domain__permissions") .filter(**filter_condition) .order_by(*sort_fields) ) @@ -185,7 +186,7 @@ def write_csv( continue total_body_rows.append(rows) - update_columns_with_domain_managers(columns, max_dm_count) + update_columns_with_domain_managers(columns, max_dm_count) write_header(writer, columns) writer.writerows(total_body_rows) From f886391093e44c83e1a425f72f6300d98d4c0324 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 21:23:06 -0500 Subject: [PATCH 3/9] Fix how we get count of dms --- 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 d325b95ea..340e49904 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -172,7 +172,7 @@ def write_csv( page = paginator.page(page_num) for domain_info in page.object_list: # Get count of all the domain managers for an account - dm_count = len(domain_info.domain.permissions) + dm_count = domain_info.domain.permissions.count() if dm_count > max_dm_count: max_dm_count = dm_count @@ -220,7 +220,7 @@ def export_data_type_to_csv(csv_file): ] filter_condition = { "domain__state__in": [ - Domain.State.READY, + Domain.State.UNKNOWN, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, ], From 9622817037ff3a9832e411eab7d2267ef652cdf4 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 21:23:43 -0500 Subject: [PATCH 4/9] test for should_write_header --- src/registrar/utility/csv_export.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 340e49904..ef531ee60 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -187,7 +187,8 @@ def write_csv( total_body_rows.append(rows) update_columns_with_domain_managers(columns, max_dm_count) - write_header(writer, columns) + if should_write_header: + write_header(writer, columns) writer.writerows(total_body_rows) @@ -220,7 +221,7 @@ def export_data_type_to_csv(csv_file): ] filter_condition = { "domain__state__in": [ - Domain.State.UNKNOWN, + Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, ], From 41f3154db353922444fd97b7fe2c2394740d9913 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 21:34:04 -0500 Subject: [PATCH 5/9] fix rows concatenation --- src/registrar/utility/csv_export.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index ef531ee60..cfba886d5 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -184,9 +184,10 @@ def write_csv( # It indicates that DomainInformation.domain is None. logger.error("csv_export -> Error when parsing row, domain was None") continue - total_body_rows.append(rows) + total_body_rows.extend(rows) - update_columns_with_domain_managers(columns, max_dm_count) + if get_domain_managers: + update_columns_with_domain_managers(columns, max_dm_count) if should_write_header: write_header(writer, columns) writer.writerows(total_body_rows) From 9ad693879a410585462c36bcf1b9f2049d5e3ab0 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 22:37:15 -0500 Subject: [PATCH 6/9] Fix column setup when domain managers --- src/registrar/utility/csv_export.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index cfba886d5..d00ce0ab2 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -45,7 +45,7 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False): +def parse_row(columns, domain_info: DomainInformation, max_dm_count, security_emails_dict=None, get_domain_managers=False): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -95,6 +95,11 @@ def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None } if get_domain_managers: + for i in range(1, max_dm_count + 1): + column_name = f"Domain manager email {i}" + if column_name not in columns: + columns.append(column_name) + # Get each domain managers email and add to list dm_emails = [dm.user.email for dm in domain.permissions.all()] @@ -127,16 +132,6 @@ def _get_security_emails(sec_contact_ids): return security_emails_dict - -def update_columns_with_domain_managers(columns, max_dm_count): - """ - Update the columns list to include "Domain manager email {#}" headers - based on the maximum domain manager count. - """ - for i in range(1, max_dm_count + 1): - columns.append(f"Domain manager email {i}") - - def write_csv( writer, columns, @@ -177,7 +172,7 @@ def write_csv( max_dm_count = dm_count try: - row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) + row = parse_row(columns, domain_info, max_dm_count, security_emails_dict, get_domain_managers) rows.append(row) except ValueError: # This should not happen. If it does, just skip this row. @@ -186,8 +181,6 @@ def write_csv( continue total_body_rows.extend(rows) - if get_domain_managers: - update_columns_with_domain_managers(columns, max_dm_count) if should_write_header: write_header(writer, columns) writer.writerows(total_body_rows) @@ -209,7 +202,7 @@ def export_data_type_to_csv(csv_file): "State", "AO", "AO email", - "Security contact email", + # "Security contact email", # For domain manager we are pass it in as a parameter below in write_body ] @@ -222,7 +215,7 @@ def export_data_type_to_csv(csv_file): ] filter_condition = { "domain__state__in": [ - Domain.State.READY, + Domain.State.UNKNOWN, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, ], From 5be47e2377a0116982f95cbbedf8fa5679728211 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 22:42:26 -0500 Subject: [PATCH 7/9] Lint --- src/registrar/utility/csv_export.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index d00ce0ab2..8d69aed05 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -45,7 +45,9 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_row(columns, domain_info: DomainInformation, max_dm_count, security_emails_dict=None, get_domain_managers=False): +def parse_row( + columns, domain_info: DomainInformation, max_dm_count, security_emails_dict=None, get_domain_managers=False +): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information @@ -132,6 +134,7 @@ def _get_security_emails(sec_contact_ids): return security_emails_dict + def write_csv( writer, columns, From 98c4dfa96af0734bd1797e8dc9d19bb4aadbf11a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 29 Feb 2024 22:48:01 -0500 Subject: [PATCH 8/9] cleanup --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 8d69aed05..b656a29e8 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -218,7 +218,7 @@ def export_data_type_to_csv(csv_file): ] filter_condition = { "domain__state__in": [ - Domain.State.UNKNOWN, + Domain.State.READY, Domain.State.DNS_NEEDED, Domain.State.ON_HOLD, ], From 1901c0dba7da4eb174a94e65df920111b3976454 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 1 Mar 2024 14:03:52 -0500 Subject: [PATCH 9/9] clean up and lint --- src/registrar/utility/csv_export.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index b656a29e8..50d580d36 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -45,13 +45,12 @@ def get_domain_infos(filter_condition, sort_fields): return domain_infos_cleaned -def parse_row( - columns, domain_info: DomainInformation, max_dm_count, security_emails_dict=None, get_domain_managers=False -): +def parse_row(columns, domain_info: DomainInformation, security_emails_dict=None, get_domain_managers=False): """Given a set of columns, generate a new row from cleaned column data""" # Domain should never be none when parsing this information if domain_info.domain is None: + logger.error("Attemting to parse row for csv exports but Domain is none in a DomainInfo") raise ValueError("Domain is none") domain = domain_info.domain # type: ignore @@ -97,11 +96,6 @@ def parse_row( } if get_domain_managers: - for i in range(1, max_dm_count + 1): - column_name = f"Domain manager email {i}" - if column_name not in columns: - columns.append(column_name) - # Get each domain managers email and add to list dm_emails = [dm.user.email for dm in domain.permissions.all()] @@ -169,13 +163,19 @@ def write_csv( rows = [] page = paginator.page(page_num) for domain_info in page.object_list: + # Get count of all the domain managers for an account - dm_count = domain_info.domain.permissions.count() - if dm_count > max_dm_count: - max_dm_count = dm_count + if get_domain_managers: + dm_count = domain_info.domain.permissions.count() + if dm_count > max_dm_count: + max_dm_count = dm_count + for i in range(1, max_dm_count + 1): + column_name = f"Domain manager email {i}" + if column_name not in columns: + columns.append(column_name) try: - row = parse_row(columns, domain_info, max_dm_count, security_emails_dict, get_domain_managers) + row = parse_row(columns, domain_info, security_emails_dict, get_domain_managers) rows.append(row) except ValueError: # This should not happen. If it does, just skip this row. @@ -205,7 +205,7 @@ def export_data_type_to_csv(csv_file): "State", "AO", "AO email", - # "Security contact email", + "Security contact email", # For domain manager we are pass it in as a parameter below in write_body ]