From 061a0961fb6d019725d2d0ebc98f3532b47d783f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:10:21 -0600 Subject: [PATCH 01/65] Create skeleton --- .../commands/create_federal_portfolio.py | 148 ++++++++++++++++++ .../transfer_transition_domains_to_domains.py | 2 +- src/registrar/models/user.py | 6 + 3 files changed, 155 insertions(+), 1 deletion(-) create mode 100644 src/registrar/management/commands/create_federal_portfolio.py diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py new file mode 100644 index 0000000000..562e468a36 --- /dev/null +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -0,0 +1,148 @@ +"""Loads files from /tmp into our sandboxes""" + +import argparse +import logging +from django.core.management import BaseCommand, CommandError +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User, SeniorOfficial +from django.db.models import Q + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Creates a federal portfolio given a FederalAgency name" + + def add_arguments(self, parser): + """Add our arguments.""" + parser.add_argument( + "agency_name", + help="The name of the FederalAgency to add", + ) + parser.add_argument( + "--parse_requests", + action=argparse.BooleanOptionalAction + help="Adds portfolio to DomainRequests", + ) + parser.add_argument( + "--parse_domains", + action=argparse.BooleanOptionalAction + help="Adds portfolio to DomainInformation", + ) + + def handle(self, agency_name, **options): + parse_requests = options.get("parse_requests") + parse_domains = options.get("parse_domains") + + if not parse_requests and not parse_domains: + raise CommandError("You must specify at least one of --parse_requests or --parse_domains.") + + agencies = FederalAgency.objects.filter(agency__iexact=agency_name) + + # TODO - maybe we can add an option here to add this if it doesn't exist? + if not agencies.exists(): + raise ValueError( + f"Cannot find the federal agency '{agency_name}' in our database. " + "The value you enter for `agency_name` must be " + "prepopulated in the FederalAgency table before proceeding." + ) + + # There should be a one-to-one relationship between the name and the agency. + federal_agency = agencies.get() + + portfolio = self.create_or_modify_portfolio(federal_agency) + self.create_suborganizations(portfolio, federal_agency) + + if parse_requests: + self.handle_portfolio_requests(portfolio, federal_agency) + + if parse_domains: + self.handle_portfolio_domains(portfolio, federal_agency) + + def create_or_modify_portfolio(self, federal_agency): + # TODO - state_territory, city, etc fields??? + portfolio_args = { + "organization_name": federal_agency.agency, + "organization_type": federal_agency.federal_type, + "senior_official": getattr(federal_agency, "so_federal_agency", None), + "creator": User.get_default_user(), + "notes": "Auto-generated record", + } + + # Create the Portfolio value if it doesn't exist + existing_portfolio = Portfolio.objects.filter(organization_name=federal_agency.agency) + if not existing_portfolio.exists(): + portfolio = Portfolio.objects.create(**portfolio_args) + message = f"Created portfolio '{federal_agency.agency}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + else: + if len(existing_portfolio) > 1: + raise ValueError(f"Could not update portfolio '{federal_agency.agency}': multiple records exist.") + + # TODO a dialog to confirm / deny doing this + existing_portfolio.update(**portfolio_args) + message = f"Modified portfolio '{federal_agency.agency}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + + return portfolio + + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): + non_federal_agency = FederalAgency.objects.get(agency="Non-Federal Agency") + valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency).exclude( + Q(federal_agency=non_federal_agency) | Q(federal_agency__isnull=True) + ) + + org_names = valid_agencies.values_list("organization_name", flat=True) + if len(org_names) < 1: + message =f"No suborganizations found for {federal_agency.agency}" + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + return + + # Check if we need to update any existing suborgs first. + # This step is optional. + existing_suborgs = Suborganization.objects.filter(name__in=org_names) + if len(existing_suborgs) > 1: + # TODO - we need a prompt here if any are found + for org in existing_suborgs: + org.portfolio = portfolio + + Suborganization.objects.bulk_update(existing_suborgs, ["portfolio"]) + message = f"Updated {len(existing_suborgs)} suborganizations" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + + # Add any suborgs that don't presently exist + suborgs = [] + for name in org_names: + if name not in existing_suborgs: + suborg = Suborganization( + name=name, + portfolio=portfolio, + ) + suborgs.append(suborg) + + Suborganization.objects.bulk_create(suborgs) + + message = f"Added {len(org_names)} suborganizations..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) + + for domain_request in domain_requests: + domain_request.portfolio = portfolio + + DomainRequest.objects.bulk_update(domain_request, ["portfolio"]) + + message = f"Added portfolio to {len(domain_requests)} domain requests" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): + domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + + for domain_info in domain_infos: + domain_info.portfolio = portfolio + + DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) + + message = f"Added portfolio to {len(domain_infos)} domains" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) diff --git a/src/registrar/management/commands/transfer_transition_domains_to_domains.py b/src/registrar/management/commands/transfer_transition_domains_to_domains.py index 615df50a56..acb886b922 100644 --- a/src/registrar/management/commands/transfer_transition_domains_to_domains.py +++ b/src/registrar/management/commands/transfer_transition_domains_to_domains.py @@ -423,7 +423,7 @@ def create_new_domain_info( valid_fed_type = fed_type in fed_choices valid_fed_agency = fed_agency in agency_choices - default_creator, _ = User.objects.get_or_create(username="System") + default_creator, _ = User.get_default_user() new_domain_info_data = { "domain": domain, diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index a7ea1e14ad..88e49a9d9e 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -157,6 +157,12 @@ def __str__(self): else: return self.username + @classmethod + def get_default_user(cls): + """Returns the default "system" user""" + default_creator, _ = User.objects.get_or_create(username="System") + return default_creator + def restrict_user(self): self.status = self.RESTRICTED self.save() From e5b1b5a87a53def67d28b324d0edbf368537038f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:28:23 -0600 Subject: [PATCH 02/65] Fix bugs --- .../commands/create_federal_portfolio.py | 34 +++++++++++++------ 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 562e468a36..5c9f960231 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -21,12 +21,12 @@ def add_arguments(self, parser): ) parser.add_argument( "--parse_requests", - action=argparse.BooleanOptionalAction + action=argparse.BooleanOptionalAction, help="Adds portfolio to DomainRequests", ) parser.add_argument( "--parse_domains", - action=argparse.BooleanOptionalAction + action=argparse.BooleanOptionalAction, help="Adds portfolio to DomainInformation", ) @@ -62,19 +62,24 @@ def handle(self, agency_name, **options): def create_or_modify_portfolio(self, federal_agency): # TODO - state_territory, city, etc fields??? portfolio_args = { + "federal_agency": federal_agency, "organization_name": federal_agency.agency, - "organization_type": federal_agency.federal_type, - "senior_official": getattr(federal_agency, "so_federal_agency", None), + "organization_type": DomainRequest.OrganizationChoices.FEDERAL, "creator": User.get_default_user(), "notes": "Auto-generated record", } + senior_official = federal_agency.so_federal_agency + if senior_official.exists(): + portfolio_args["senior_official"] = senior_official.first() + # Create the Portfolio value if it doesn't exist existing_portfolio = Portfolio.objects.filter(organization_name=federal_agency.agency) if not existing_portfolio.exists(): portfolio = Portfolio.objects.create(**portfolio_args) message = f"Created portfolio '{federal_agency.agency}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + return portfolio else: if len(existing_portfolio) > 1: raise ValueError(f"Could not update portfolio '{federal_agency.agency}': multiple records exist.") @@ -82,9 +87,8 @@ def create_or_modify_portfolio(self, federal_agency): # TODO a dialog to confirm / deny doing this existing_portfolio.update(**portfolio_args) message = f"Modified portfolio '{federal_agency.agency}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - - return portfolio + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + return existing_portfolio def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): non_federal_agency = FederalAgency.objects.get(agency="Non-Federal Agency") @@ -127,22 +131,30 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) + if len(domain_requests) < 1: + message = f"Portfolios not added to domain requests: no valid records found" + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + return for domain_request in domain_requests: domain_request.portfolio = portfolio - DomainRequest.objects.bulk_update(domain_request, ["portfolio"]) - + DomainRequest.objects.bulk_update(domain_requests, ["portfolio"]) message = f"Added portfolio to {len(domain_requests)} domain requests" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + if len(domain_infos) < 1: + message = f"Portfolios not added to domains: no valid records found" + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + return + for domain_info in domain_infos: domain_info.portfolio = portfolio DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) message = f"Added portfolio to {len(domain_infos)} domains" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) From d35acc8a306e52614c0e6a0c805e54924fcf4806 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:54:12 -0600 Subject: [PATCH 03/65] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 48 +++++++++++++++---- 1 file changed, 39 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5c9f960231..68e625bc95 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -77,18 +77,18 @@ def create_or_modify_portfolio(self, federal_agency): existing_portfolio = Portfolio.objects.filter(organization_name=federal_agency.agency) if not existing_portfolio.exists(): portfolio = Portfolio.objects.create(**portfolio_args) - message = f"Created portfolio '{federal_agency.agency}'" + message = f"Created portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) return portfolio else: if len(existing_portfolio) > 1: - raise ValueError(f"Could not update portfolio '{federal_agency.agency}': multiple records exist.") + raise ValueError(f"Could not update portfolio '{portfolio}': multiple records exist.") # TODO a dialog to confirm / deny doing this existing_portfolio.update(**portfolio_args) message = f"Modified portfolio '{federal_agency.agency}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - return existing_portfolio + return existing_portfolio.get() def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): non_federal_agency = FederalAgency.objects.get(agency="Non-Federal Agency") @@ -115,20 +115,50 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) # Add any suborgs that don't presently exist + excluded_org_names = existing_suborgs.values_list("name", flat=True) suborgs = [] for name in org_names: - if name not in existing_suborgs: - suborg = Suborganization( - name=name, - portfolio=portfolio, - ) - suborgs.append(suborg) + if name and name not in excluded_org_names: + if portfolio.organization_name and name.lower() == portfolio.organization_name.lower(): + # If the suborg name is the name that currently exists, + # thats not a suborg - thats the portfolio itself! + # In this case, we can use this as an opportunity to update + # address information and the like + self._update_portfolio_location_details(portfolio, valid_agencies.get(organization_name=name)) + else: + suborg = Suborganization( + name=name, + portfolio=portfolio, + ) + suborgs.append(suborg) Suborganization.objects.bulk_create(suborgs) + # TODO - this is inaccurate when the suborg name does not equal the org name - i.e. if the + # record exists already as well message = f"Added {len(org_names)} suborganizations..." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + # TODO rename + def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): + location_props = [ + "address_line1", + "address_line2", + "city", + "state_territory", + "zipcode", + "urbanization", + ] + + for prop_name in location_props: + # Copy the value from the domain info object to the portfolio object + value = getattr(domain_info, prop_name) + setattr(portfolio, prop_name, value) + portfolio.save() + + message = f"Updated location details on portfolio '{portfolio}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) if len(domain_requests) < 1: From 8e6115acf0fe294cf9a4abc06fd85dd4e4705914 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:00:10 -0600 Subject: [PATCH 04/65] Add some better logging and options --- .../commands/create_federal_portfolio.py | 86 +++++++++++++------ 1 file changed, 59 insertions(+), 27 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 68e625bc95..6d62a645bc 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -60,7 +60,9 @@ def handle(self, agency_name, **options): self.handle_portfolio_domains(portfolio, federal_agency) def create_or_modify_portfolio(self, federal_agency): - # TODO - state_territory, city, etc fields??? + """Tries to create a portfolio record based off of a federal agency. + If the record already exists, we prompt the user to proceed then + update the record.""" portfolio_args = { "federal_agency": federal_agency, "organization_name": federal_agency.agency, @@ -81,38 +83,44 @@ def create_or_modify_portfolio(self, federal_agency): TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) return portfolio else: + + proceed = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + info_to_inspect=f"""The given portfolio '{federal_agency.agency}' already exists in our DB. + If you cancel, the rest of the script will still execute but this record will not update. + """, + prompt_title="Do you wish to modify this record?", + ) + if not proceed: + if len(existing_portfolio) > 1: + raise ValueError(f"Could not use portfolio '{federal_agency.agency}': multiple records exist.") + else: + # Just return the portfolio object without modifying it + return existing_portfolio.get() + if len(existing_portfolio) > 1: - raise ValueError(f"Could not update portfolio '{portfolio}': multiple records exist.") + raise ValueError(f"Could not update portfolio '{federal_agency.agency}': multiple records exist.") - # TODO a dialog to confirm / deny doing this existing_portfolio.update(**portfolio_args) - message = f"Modified portfolio '{federal_agency.agency}'" + message = f"Modified portfolio '{existing_portfolio.first()}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) return existing_portfolio.get() def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): - non_federal_agency = FederalAgency.objects.get(agency="Non-Federal Agency") - valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency).exclude( - Q(federal_agency=non_federal_agency) | Q(federal_agency__isnull=True) - ) - + """Given a list of organization_names on DomainInformation objects (filtered by agency), + create multiple Suborganizations tied to the given portfolio""" + valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency) org_names = valid_agencies.values_list("organization_name", flat=True) if len(org_names) < 1: - message =f"No suborganizations found for {federal_agency.agency}" + message =f"No suborganizations found for {federal_agency}" TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) return # Check if we need to update any existing suborgs first. # This step is optional. existing_suborgs = Suborganization.objects.filter(name__in=org_names) - if len(existing_suborgs) > 1: - # TODO - we need a prompt here if any are found - for org in existing_suborgs: - org.portfolio = portfolio - - Suborganization.objects.bulk_update(existing_suborgs, ["portfolio"]) - message = f"Updated {len(existing_suborgs)} suborganizations" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + if len(existing_suborgs) > 0: + self._update_existing_suborganizations(portfolio, existing_suborgs) # Add any suborgs that don't presently exist excluded_org_names = existing_suborgs.values_list("name", flat=True) @@ -132,15 +140,39 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA ) suborgs.append(suborg) - Suborganization.objects.bulk_create(suborgs) + if len(org_names) > 1: + Suborganization.objects.bulk_create(suborgs) + message = f"Added {len(suborgs)} suborganizations" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + else: + message =f"No suborganizations added" + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + + def _update_existing_suborganizations(self, portfolio, orgs_to_update): + proceed = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + info_to_inspect=f"""Some suborganizations already exist in our DB. + If you cancel, the rest of the script will still execute but these records will not update. + + ==Proposed Changes== + The following suborgs will be updated: {[org.name for org in orgs_to_update]} + """, + prompt_title="Do you wish to modify existing suborganizations?", + ) + if not proceed: + return + + for org in orgs_to_update: + org.portfolio = portfolio + + Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) + message = f"Updated {len(orgs_to_update)} suborganizations" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - # TODO - this is inaccurate when the suborg name does not equal the org name - i.e. if the - # record exists already as well - message = f"Added {len(org_names)} suborganizations..." - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - # TODO rename def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): + """Adds location information to the given portfolio based off of the values in + DomainInformation""" location_props = [ "address_line1", "address_line2", @@ -168,9 +200,9 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa for domain_request in domain_requests: domain_request.portfolio = portfolio - + DomainRequest.objects.bulk_update(domain_requests, ["portfolio"]) - message = f"Added portfolio to {len(domain_requests)} domain requests" + message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): @@ -186,5 +218,5 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) - message = f"Added portfolio to {len(domain_infos)} domains" + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) From 8d324a81d1ccd061e75befbdb2b8b27c966b557f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:41:52 -0600 Subject: [PATCH 05/65] linting --- .../commands/create_federal_portfolio.py | 21 +++++++++---------- .../transfer_transition_domains_to_domains.py | 2 +- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 6d62a645bc..29697723b9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -4,8 +4,8 @@ import logging from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper -from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User, SeniorOfficial -from django.db.models import Q +from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User + logger = logging.getLogger(__name__) @@ -55,7 +55,7 @@ def handle(self, agency_name, **options): if parse_requests: self.handle_portfolio_requests(portfolio, federal_agency) - + if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) @@ -83,10 +83,10 @@ def create_or_modify_portfolio(self, federal_agency): TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) return portfolio else: - + proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - info_to_inspect=f"""The given portfolio '{federal_agency.agency}' already exists in our DB. + info_to_inspect=f"""The given portfolio '{federal_agency.agency}' already exists in our DB. If you cancel, the rest of the script will still execute but this record will not update. """, prompt_title="Do you wish to modify this record?", @@ -112,7 +112,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency) org_names = valid_agencies.values_list("organization_name", flat=True) if len(org_names) < 1: - message =f"No suborganizations found for {federal_agency}" + message = f"No suborganizations found for {federal_agency}" TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) return @@ -145,7 +145,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA message = f"Added {len(suborgs)} suborganizations" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) else: - message =f"No suborganizations added" + message = "No suborganizations added" TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) def _update_existing_suborganizations(self, portfolio, orgs_to_update): @@ -169,9 +169,8 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): message = f"Updated {len(orgs_to_update)} suborganizations" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): - """Adds location information to the given portfolio based off of the values in + """Adds location information to the given portfolio based off of the values in DomainInformation""" location_props = [ "address_line1", @@ -194,7 +193,7 @@ def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) if len(domain_requests) < 1: - message = f"Portfolios not added to domain requests: no valid records found" + message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return @@ -209,7 +208,7 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) if len(domain_infos) < 1: - message = f"Portfolios not added to domains: no valid records found" + message = "Portfolios not added to domains: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return diff --git a/src/registrar/management/commands/transfer_transition_domains_to_domains.py b/src/registrar/management/commands/transfer_transition_domains_to_domains.py index acb886b922..727db6dabc 100644 --- a/src/registrar/management/commands/transfer_transition_domains_to_domains.py +++ b/src/registrar/management/commands/transfer_transition_domains_to_domains.py @@ -423,7 +423,7 @@ def create_new_domain_info( valid_fed_type = fed_type in fed_choices valid_fed_agency = fed_agency in agency_choices - default_creator, _ = User.get_default_user() + default_creator = User.get_default_user() new_domain_info_data = { "domain": domain, From bfbce5a5c5703e73bbec26b2c822ee4d71e9778a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 10:48:21 -0600 Subject: [PATCH 06/65] Comments --- .../commands/create_federal_portfolio.py | 18 +++++++++++++++-- .../tests/test_management_scripts.py | 20 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 29697723b9..00da3b40b6 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -149,6 +149,10 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) def _update_existing_suborganizations(self, portfolio, orgs_to_update): + """ + Update existing suborganizations with new portfolio. + Prompts for user confirmation before proceeding. + """ proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, info_to_inspect=f"""Some suborganizations already exist in our DB. @@ -170,8 +174,10 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): - """Adds location information to the given portfolio based off of the values in - DomainInformation""" + """ + Update portfolio location details based on DomainInformation. + Copies relevant fields and saves the portfolio. + """ location_props = [ "address_line1", "address_line2", @@ -191,6 +197,10 @@ def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + """ + Associate portfolio with domain requests for a federal agency. + Updates all relevant domain request records. + """ domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) if len(domain_requests) < 1: message = "Portfolios not added to domain requests: no valid records found" @@ -205,6 +215,10 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): + """ + Associate portfolio with domains for a federal agency. + Updates all relevant domain information records. + """ domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) if len(domain_infos) < 1: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 1958454f5b..9dab760632 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1408,3 +1408,23 @@ def test_populate_federal_agency_initials_and_fceb_missing_agency(self): missing_agency.refresh_from_db() self.assertIsNone(missing_agency.initials) self.assertIsNone(missing_agency.is_fceb) + + +class TestCreateFederalPortfolio(TestCase): + def setUp(self): + self.csv_path = "registrar/tests/data/fake_federal_cio.csv" + + # Create test FederalAgency objects + self.agency1, _ = FederalAgency.objects.get_or_create(agency="American Battle Monuments Commission") + self.agency2, _ = FederalAgency.objects.get_or_create(agency="Advisory Council on Historic Preservation") + self.agency3, _ = FederalAgency.objects.get_or_create(agency="AMTRAK") + self.agency4, _ = FederalAgency.objects.get_or_create(agency="John F. Kennedy Center for Performing Arts") + + def tearDown(self): + SeniorOfficial.objects.all().delete() + FederalAgency.objects.all().delete() + + # == create_or_modify_portfolio tests == # + # == create_suborganizations tests == # + # == handle_portfolio_requests tests == # + # == handle_portfolio_domains tests == # From b162ae8d7fd14475bac6853f658955f9edd698e1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:27:20 -0600 Subject: [PATCH 07/65] Simplify logic --- .../commands/create_federal_portfolio.py | 128 +++++++----------- 1 file changed, 50 insertions(+), 78 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 00da3b40b6..a0145418f7 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -14,7 +14,11 @@ class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" def add_arguments(self, parser): - """Add our arguments.""" + """Add three arguments: + 1. agency_name => the value of FederalAgency.agency + 2. --parse_requests => if true, adds the given portfolio to each related DomainRequest + 3. --parse_domains => if true, adds the given portfolio to each related DomainInformation + """ parser.add_argument( "agency_name", help="The name of the FederalAgency to add", @@ -37,19 +41,14 @@ def handle(self, agency_name, **options): if not parse_requests and not parse_domains: raise CommandError("You must specify at least one of --parse_requests or --parse_domains.") - agencies = FederalAgency.objects.filter(agency__iexact=agency_name) - - # TODO - maybe we can add an option here to add this if it doesn't exist? - if not agencies.exists(): + federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first() + if not federal_agency: raise ValueError( f"Cannot find the federal agency '{agency_name}' in our database. " "The value you enter for `agency_name` must be " "prepopulated in the FederalAgency table before proceeding." ) - # There should be a one-to-one relationship between the name and the agency. - federal_agency = agencies.get() - portfolio = self.create_or_modify_portfolio(federal_agency) self.create_suborganizations(portfolio, federal_agency) @@ -60,9 +59,7 @@ def handle(self, agency_name, **options): self.handle_portfolio_domains(portfolio, federal_agency) def create_or_modify_portfolio(self, federal_agency): - """Tries to create a portfolio record based off of a federal agency. - If the record already exists, we prompt the user to proceed then - update the record.""" + """Creates or modifies a portfolio record based on a federal agency.""" portfolio_args = { "federal_agency": federal_agency, "organization_name": federal_agency.agency, @@ -71,19 +68,15 @@ def create_or_modify_portfolio(self, federal_agency): "notes": "Auto-generated record", } - senior_official = federal_agency.so_federal_agency - if senior_official.exists(): - portfolio_args["senior_official"] = senior_official.first() + if federal_agency.so_federal_agency.exists(): + portfolio_args["senior_official"] = federal_agency.so_federal_agency.first() + + portfolio, created = Portfolio.objects.get_or_create(**portfolio_args) - # Create the Portfolio value if it doesn't exist - existing_portfolio = Portfolio.objects.filter(organization_name=federal_agency.agency) - if not existing_portfolio.exists(): - portfolio = Portfolio.objects.create(**portfolio_args) + if created: message = f"Created portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - return portfolio else: - proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, info_to_inspect=f"""The given portfolio '{federal_agency.agency}' already exists in our DB. @@ -91,62 +84,44 @@ def create_or_modify_portfolio(self, federal_agency): """, prompt_title="Do you wish to modify this record?", ) - if not proceed: - if len(existing_portfolio) > 1: - raise ValueError(f"Could not use portfolio '{federal_agency.agency}': multiple records exist.") - else: - # Just return the portfolio object without modifying it - return existing_portfolio.get() - - if len(existing_portfolio) > 1: - raise ValueError(f"Could not update portfolio '{federal_agency.agency}': multiple records exist.") - - existing_portfolio.update(**portfolio_args) - message = f"Modified portfolio '{existing_portfolio.first()}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - return existing_portfolio.get() + if proceed: + for key, value in portfolio_args.items(): + setattr(portfolio, key, value) + portfolio.save() + message = f"Modified portfolio '{portfolio}'" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + return portfolio def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): - """Given a list of organization_names on DomainInformation objects (filtered by agency), - create multiple Suborganizations tied to the given portfolio""" - valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency) - org_names = valid_agencies.values_list("organization_name", flat=True) - if len(org_names) < 1: - message = f"No suborganizations found for {federal_agency}" - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" + valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency, organization_name__isnull=False) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + + if not org_names: + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, f"No suborganizations found for {federal_agency}") return - # Check if we need to update any existing suborgs first. - # This step is optional. + # Check if we need to update any existing suborgs first. This step is optional. existing_suborgs = Suborganization.objects.filter(name__in=org_names) - if len(existing_suborgs) > 0: + if existing_suborgs.exists(): self._update_existing_suborganizations(portfolio, existing_suborgs) - # Add any suborgs that don't presently exist - excluded_org_names = existing_suborgs.values_list("name", flat=True) - suborgs = [] - for name in org_names: - if name and name not in excluded_org_names: - if portfolio.organization_name and name.lower() == portfolio.organization_name.lower(): - # If the suborg name is the name that currently exists, - # thats not a suborg - thats the portfolio itself! - # In this case, we can use this as an opportunity to update - # address information and the like - self._update_portfolio_location_details(portfolio, valid_agencies.get(organization_name=name)) - else: - suborg = Suborganization( - name=name, - portfolio=portfolio, - ) - suborgs.append(suborg) - - if len(org_names) > 1: - Suborganization.objects.bulk_create(suborgs) - message = f"Added {len(suborgs)} suborganizations" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + # Create new suborgs, as long as they don't exist in the db already + new_suborgs = [] + for name in org_names - set(existing_suborgs.values_list("name", flat=True)): + if name.lower() == portfolio.organization_name.lower(): + # If the suborg name is a portfolio name that currently exists, thats not a suborg - thats the portfolio itself! + # In this case, we can use this as an opportunity to update address information. + self._update_portfolio_location_details(portfolio, valid_agencies.filter(organization_name=name).first()) + else: + new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) + + if new_suborgs: + Suborganization.objects.bulk_create(new_suborgs) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Added {len(new_suborgs)} suborganizations") else: - message = "No suborganizations added" - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") def _update_existing_suborganizations(self, portfolio, orgs_to_update): """ @@ -163,15 +138,13 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): """, prompt_title="Do you wish to modify existing suborganizations?", ) - if not proceed: - return + if proceed: + for org in orgs_to_update: + org.portfolio = portfolio - for org in orgs_to_update: - org.portfolio = portfolio - - Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) - message = f"Updated {len(orgs_to_update)} suborganizations" - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) + message = f"Updated {len(orgs_to_update)} suborganizations" + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): """ @@ -191,8 +164,8 @@ def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: # Copy the value from the domain info object to the portfolio object value = getattr(domain_info, prop_name) setattr(portfolio, prop_name, value) - portfolio.save() + portfolio.save() message = f"Updated location details on portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) @@ -230,6 +203,5 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal domain_info.portfolio = portfolio DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) From b27913f124fa5a202fcb7fd733d0ae7bb06d6232 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:34:09 -0600 Subject: [PATCH 08/65] Avoid using an early return (consistency) --- .../commands/create_federal_portfolio.py | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index a0145418f7..da2c2327dc 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -175,17 +175,16 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa Updates all relevant domain request records. """ domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) - if len(domain_requests) < 1: + if not domain_requests.exists(): message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - return - - for domain_request in domain_requests: - domain_request.portfolio = portfolio + else: + for domain_request in domain_requests: + domain_request.portfolio = portfolio - DomainRequest.objects.bulk_update(domain_requests, ["portfolio"]) - message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + DomainRequest.objects.bulk_update(domain_requests, ["portfolio"]) + message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): """ @@ -193,15 +192,14 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal Updates all relevant domain information records. """ domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) - - if len(domain_infos) < 1: + if not domain_infos.exists(): message = "Portfolios not added to domains: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return + else: + for domain_info in domain_infos: + domain_info.portfolio = portfolio - for domain_info in domain_infos: - domain_info.portfolio = portfolio - - DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) From 12eab5c44fcac0d945cfc5a45d17764abfdab71d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:34:35 -0600 Subject: [PATCH 09/65] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index da2c2327dc..f7174ac680 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -195,7 +195,6 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal if not domain_infos.exists(): message = "Portfolios not added to domains: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - return else: for domain_info in domain_infos: domain_info.portfolio = portfolio From 11d662aa3cd888ae0e07601da72ef60f60bb3942 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:42:32 -0600 Subject: [PATCH 10/65] use defaults --- .../management/commands/create_federal_portfolio.py | 5 ++++- src/registrar/tests/test_management_scripts.py | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index f7174ac680..a5b4322473 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -71,7 +71,10 @@ def create_or_modify_portfolio(self, federal_agency): if federal_agency.so_federal_agency.exists(): portfolio_args["senior_official"] = federal_agency.so_federal_agency.first() - portfolio, created = Portfolio.objects.get_or_create(**portfolio_args) + portfolio, created = Portfolio.objects.get_or_create( + organization_name=portfolio_args.get("organization_name"), + defaults=portfolio_args + ) if created: message = f"Created portfolio '{portfolio}'" diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 9dab760632..f9c544bfeb 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1428,3 +1428,6 @@ def tearDown(self): # == create_suborganizations tests == # # == handle_portfolio_requests tests == # # == handle_portfolio_domains tests == # + # test for parse_requests + # test for parse_domains + # test for both From 91e62a6cdfcb24710c9b1dc6cf868f18f17e85b9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 14:18:38 -0600 Subject: [PATCH 11/65] Unit tests --- docs/operations/data_migration.md | 42 ++++++ .../commands/create_federal_portfolio.py | 28 +++- src/registrar/tests/common.py | 7 +- .../tests/test_management_scripts.py | 141 +++++++++++++++--- 4 files changed, 194 insertions(+), 24 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 5914eb1795..c261e7a066 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -838,3 +838,45 @@ Example: `cf ssh getgov-za` ### Running locally ```docker-compose exec app ./manage.py populate_domain_request_dates``` + +## Create federal portfolio +This script takes the name of a `FederalAgency` (like 'AMTRAK') and does the following: +1. Creates the portfolio record based off of data on the federal agency object itself +2. Creates suborganizations from existing DomainInformation records +3. Associates the SeniorOfficial record (if it exists) +4. Adds this portfolio to DomainInformation / DomainRequests or both + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Upload your csv to the desired sandbox +[Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. + +#### Step 5: Running the script +```./manage.py create_federal_portfolio "{federal_agency_name}" --parse_requests --parse_domains``` + +Example: `./manage.py create_federal_portfolio "AMTRAK" --parse_requests --parse_domains` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --parse_requests --parse_domains``` + +##### Parameters +| | Parameter | Description | +|:-:|:-------------------------- |:-------------------------------------------------------------------------------------------| +| 1 | **federal_agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | +| 2 | **parse_requests** | Optional. If True, then the created portfolio is added to all related DomainRequests. | +| 3 | **parse_domains** | Optional. If True, then the created portfolio is added to all related Domains. | + +Note: While you can specify both at the same time, you must specify either --parse_requests or --parse_domains. You cannot run the script without defining one or the other. diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index a5b4322473..4ca6ba17fd 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -72,8 +72,7 @@ def create_or_modify_portfolio(self, federal_agency): portfolio_args["senior_official"] = federal_agency.so_federal_agency.first() portfolio, created = Portfolio.objects.get_or_create( - organization_name=portfolio_args.get("organization_name"), - defaults=portfolio_args + organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args ) if created: @@ -88,6 +87,15 @@ def create_or_modify_portfolio(self, federal_agency): prompt_title="Do you wish to modify this record?", ) if proceed: + + # Don't override the creator and notes fields + if portfolio.creator: + portfolio_args.pop("creator") + + if portfolio.notes: + portfolio_args.pop("notes") + + # Update everything else for key, value in portfolio_args.items(): setattr(portfolio, key, value) portfolio.save() @@ -98,11 +106,15 @@ def create_or_modify_portfolio(self, federal_agency): def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - valid_agencies = DomainInformation.objects.filter(federal_agency=federal_agency, organization_name__isnull=False) + valid_agencies = DomainInformation.objects.filter( + federal_agency=federal_agency, organization_name__isnull=False + ) org_names = set(valid_agencies.values_list("organization_name", flat=True)) if not org_names: - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, f"No suborganizations found for {federal_agency}") + TerminalHelper.colorful_logger( + logger.warning, TerminalColors.YELLOW, f"No suborganizations found for {federal_agency}" + ) return # Check if we need to update any existing suborgs first. This step is optional. @@ -116,13 +128,17 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA if name.lower() == portfolio.organization_name.lower(): # If the suborg name is a portfolio name that currently exists, thats not a suborg - thats the portfolio itself! # In this case, we can use this as an opportunity to update address information. - self._update_portfolio_location_details(portfolio, valid_agencies.filter(organization_name=name).first()) + self._update_portfolio_location_details( + portfolio, valid_agencies.filter(organization_name=name).first() + ) else: new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Added {len(new_suborgs)} suborganizations") + TerminalHelper.colorful_logger( + logger.info, TerminalColors.OKGREEN, f"Added {len(new_suborgs)} suborganizations" + ) else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index bcd45f103a..78cd1aafbb 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -911,6 +911,8 @@ def completed_domain_request( # noqa federal_type=None, action_needed_reason=None, portfolio=None, + organization_name=None, + city=None, ): """A completed domain request.""" if not user: @@ -954,7 +956,7 @@ def completed_domain_request( # noqa federal_type="executive", purpose="Purpose of the site", is_policy_acknowledged=True, - organization_name="Testorg", + organization_name=organization_name if organization_name else "Testorg", address_line1="address 1", address_line2="address 2", state_territory="NY", @@ -984,6 +986,9 @@ def completed_domain_request( # noqa if portfolio: domain_request_kwargs["portfolio"] = portfolio + if city: + domain_request_kwargs["city"] = city + domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index f9c544bfeb..77822a022b 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1,4 +1,5 @@ import copy +import boto3_mocking # type: ignore from datetime import date, datetime, time from django.core.management import call_command from django.test import TestCase, override_settings @@ -8,6 +9,7 @@ from django.utils.module_loading import import_string import logging import pyzipper +from django.core.management.base import CommandError from registrar.management.commands.clean_tables import Command as CleanTablesCommand from registrar.management.commands.export_tables import Command as ExportTablesCommand from registrar.models import ( @@ -23,14 +25,17 @@ VerifiedByStaff, PublicContact, FederalAgency, + Portfolio, + Suborganization, ) import tablib from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common -from .common import MockEppLib, less_console_noise, completed_domain_request +from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient from api.tests.common import less_console_noise_decorator + logger = logging.getLogger(__name__) @@ -1411,23 +1416,125 @@ def test_populate_federal_agency_initials_and_fceb_missing_agency(self): class TestCreateFederalPortfolio(TestCase): - def setUp(self): - self.csv_path = "registrar/tests/data/fake_federal_cio.csv" - # Create test FederalAgency objects - self.agency1, _ = FederalAgency.objects.get_or_create(agency="American Battle Monuments Commission") - self.agency2, _ = FederalAgency.objects.get_or_create(agency="Advisory Council on Historic Preservation") - self.agency3, _ = FederalAgency.objects.get_or_create(agency="AMTRAK") - self.agency4, _ = FederalAgency.objects.get_or_create(agency="John F. Kennedy Center for Performing Arts") + @less_console_noise_decorator + def setUp(self): + self.mock_client = MockSESClient() + self.user = User.objects.create(username="testuser") + self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + self.domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + city="WrongCity", + ) + self.domain_request.approve() + self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get() + + self.domain_request_2 = completed_domain_request( + name="sock@igorville.org", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + organization_name="Test Federal Agency", + city="Block", + ) + self.domain_request_2.approve() + self.domain_info_2 = DomainInformation.objects.filter(domain_request=self.domain_request_2).get() def tearDown(self): - SeniorOfficial.objects.all().delete() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + Suborganization.objects.all().delete() + Portfolio.objects.all().delete() FederalAgency.objects.all().delete() - - # == create_or_modify_portfolio tests == # - # == create_suborganizations tests == # - # == handle_portfolio_requests tests == # - # == handle_portfolio_domains tests == # - # test for parse_requests - # test for parse_domains - # test for both + User.objects.all().delete() + + @less_console_noise_decorator + def run_create_federal_portfolio(self, agency_name, parse_requests=False, parse_domains=False): + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", + return_value=True, + ): + call_command( + "create_federal_portfolio", agency_name, parse_requests=parse_requests, parse_domains=parse_domains + ) + + def test_create_or_modify_portfolio(self): + self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + + portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) + self.assertEqual(portfolio.organization_name, self.federal_agency.agency) + self.assertEqual(portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL) + self.assertEqual(portfolio.creator, User.get_default_user()) + self.assertEqual(portfolio.notes, "Auto-generated record") + + # Test the suborgs + suborganizations = Suborganization.objects.filter(portfolio__federal_agency=self.federal_agency) + self.assertEqual(suborganizations.count(), 1) + self.assertEqual(suborganizations.first().name, "Testorg") + + # Test other address information + self.assertEqual(portfolio.address_line1, "address 1") + self.assertEqual(portfolio.city, "Block") + self.assertEqual(portfolio.state_territory, "NY") + self.assertEqual(portfolio.zipcode, "10002") + + def test_handle_portfolio_requests(self): + self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + + self.domain_request.refresh_from_db() + self.assertIsNotNone(self.domain_request.portfolio) + self.assertEqual(self.domain_request.portfolio.federal_agency, self.federal_agency) + + def test_handle_portfolio_domains(self): + self.run_create_federal_portfolio("Test Federal Agency", parse_domains=True) + + self.domain_info.refresh_from_db() + self.assertIsNotNone(self.domain_info.portfolio) + self.assertEqual(self.domain_info.portfolio.federal_agency, self.federal_agency) + + def test_handle_parse_both(self): + self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True, parse_domains=True) + + self.domain_request.refresh_from_db() + self.domain_info.refresh_from_db() + self.assertIsNotNone(self.domain_request.portfolio) + self.assertIsNotNone(self.domain_info.portfolio) + self.assertEqual(self.domain_request.portfolio, self.domain_info.portfolio) + + def test_command_error_no_parse_options(self): + with self.assertRaisesRegex( + CommandError, "You must specify at least one of --parse_requests or --parse_domains." + ): + self.run_create_federal_portfolio("Test Federal Agency") + + def test_command_error_agency_not_found(self): + expected_message = ( + "Cannot find the federal agency 'Non-existent Agency' in our database. " + "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding." + ) + with self.assertRaisesRegex(ValueError, expected_message): + self.run_create_federal_portfolio("Non-existent Agency", parse_requests=True) + + def test_update_existing_portfolio(self): + # Create an existing portfolio + existing_portfolio = Portfolio.objects.create( + federal_agency=self.federal_agency, + organization_name="Test Federal Agency", + organization_type=DomainRequest.OrganizationChoices.CITY, + creator=self.user, + notes="Old notes", + ) + + self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) + + existing_portfolio.refresh_from_db() + self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) + self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL) + # Notes and creator should be untouched + self.assertEqual(existing_portfolio.notes, "Old notes") + self.assertEqual(existing_portfolio.creator, self.user) From f2288673a02851ab8714a9f2541bf516e653d16a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 29 Aug 2024 15:18:14 -0600 Subject: [PATCH 12/65] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 4ca6ba17fd..bf42977052 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -193,7 +193,14 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ - domain_requests = DomainInformation.objects.filter(federal_agency=federal_agency) + invalid_states = [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.REJECTED, + ] + domain_requests = DomainRequest.objects.filter( + federal_agency=federal_agency + ).exclude(status__in=invalid_states) if not domain_requests.exists(): message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) From b3b415b56347f5f8f8fa693ef9ca4320912f2190 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 30 Aug 2024 08:40:04 -0600 Subject: [PATCH 13/65] Remove logic for location information --- .../commands/create_federal_portfolio.py | 40 +++++-------------- src/registrar/tests/common.py | 4 -- .../tests/test_management_scripts.py | 18 ++++----- 3 files changed, 18 insertions(+), 44 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index bf42977052..219044fc57 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -98,6 +98,7 @@ def create_or_modify_portfolio(self, federal_agency): # Update everything else for key, value in portfolio_args.items(): setattr(portfolio, key, value) + portfolio.save() message = f"Modified portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) @@ -126,11 +127,13 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): if name.lower() == portfolio.organization_name.lower(): - # If the suborg name is a portfolio name that currently exists, thats not a suborg - thats the portfolio itself! - # In this case, we can use this as an opportunity to update address information. - self._update_portfolio_location_details( - portfolio, valid_agencies.filter(organization_name=name).first() + # You can use this to populate location information, when this occurs. + # However, this isn't needed for now so we can skip it. + message = ( + f"Skipping suborganization create on record '{name}'. " + f"The federal agency name is the same as the portfolio name." ) + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) @@ -165,42 +168,17 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): message = f"Updated {len(orgs_to_update)} suborganizations" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) - def _update_portfolio_location_details(self, portfolio: Portfolio, domain_info: DomainInformation): - """ - Update portfolio location details based on DomainInformation. - Copies relevant fields and saves the portfolio. - """ - location_props = [ - "address_line1", - "address_line2", - "city", - "state_territory", - "zipcode", - "urbanization", - ] - - for prop_name in location_props: - # Copy the value from the domain info object to the portfolio object - value = getattr(domain_info, prop_name) - setattr(portfolio, prop_name, value) - - portfolio.save() - message = f"Updated location details on portfolio '{portfolio}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter( - federal_agency=federal_agency - ).exclude(status__in=invalid_states) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) if not domain_requests.exists(): message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 78cd1aafbb..5e418da2b4 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -912,7 +912,6 @@ def completed_domain_request( # noqa action_needed_reason=None, portfolio=None, organization_name=None, - city=None, ): """A completed domain request.""" if not user: @@ -986,9 +985,6 @@ def completed_domain_request( # noqa if portfolio: domain_request_kwargs["portfolio"] = portfolio - if city: - domain_request_kwargs["city"] = city - domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 77822a022b..5c049ac56f 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1422,13 +1422,15 @@ def setUp(self): self.mock_client = MockSESClient() self.user = User.objects.create(username="testuser") self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + self.senior_official = SeniorOfficial.objects.create( + first_name="first", last_name="last", email="testuser@igorville.gov", federal_agency=self.federal_agency + ) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): self.domain_request = completed_domain_request( status=DomainRequest.DomainRequestStatus.IN_REVIEW, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, user=self.user, - city="WrongCity", ) self.domain_request.approve() self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get() @@ -1436,11 +1438,10 @@ def setUp(self): self.domain_request_2 = completed_domain_request( name="sock@igorville.org", status=DomainRequest.DomainRequestStatus.IN_REVIEW, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, user=self.user, organization_name="Test Federal Agency", - city="Block", ) self.domain_request_2.approve() self.domain_info_2 = DomainInformation.objects.filter(domain_request=self.domain_request_2).get() @@ -1450,6 +1451,7 @@ def tearDown(self): DomainRequest.objects.all().delete() Suborganization.objects.all().delete() Portfolio.objects.all().delete() + SeniorOfficial.objects.all().delete() FederalAgency.objects.all().delete() User.objects.all().delete() @@ -1477,11 +1479,8 @@ def test_create_or_modify_portfolio(self): self.assertEqual(suborganizations.count(), 1) self.assertEqual(suborganizations.first().name, "Testorg") - # Test other address information - self.assertEqual(portfolio.address_line1, "address 1") - self.assertEqual(portfolio.city, "Block") - self.assertEqual(portfolio.state_territory, "NY") - self.assertEqual(portfolio.zipcode, "10002") + # Test the senior official + self.assertEqual(portfolio.senior_official, self.senior_official) def test_handle_portfolio_requests(self): self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) @@ -1535,6 +1534,7 @@ def test_update_existing_portfolio(self): existing_portfolio.refresh_from_db() self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL) + # Notes and creator should be untouched self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) From 47b7ee45cc3cf3442a7386aab9aafffd5eb5db4e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:31:55 -0600 Subject: [PATCH 14/65] lint --- .../management/commands/create_federal_portfolio.py | 7 +++++-- src/registrar/tests/test_management_scripts.py | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 219044fc57..017854b914 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -81,7 +81,8 @@ def create_or_modify_portfolio(self, federal_agency): else: proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - info_to_inspect=f"""The given portfolio '{federal_agency.agency}' already exists in our DB. + info_to_inspect=f""" + The given portfolio '{federal_agency.agency}' already exists in our DB. If you cancel, the rest of the script will still execute but this record will not update. """, prompt_title="Do you wish to modify this record?", @@ -126,7 +127,9 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): - if name.lower() == portfolio.organization_name.lower(): + # Stored in variables due to linter wanting type information here + portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" + if name is not None and name.lower() == portfolio_name.lower(): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 5c049ac56f..29db8a4c59 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1466,6 +1466,7 @@ def run_create_federal_portfolio(self, agency_name, parse_requests=False, parse_ ) def test_create_or_modify_portfolio(self): + """Test portfolio creation and modification with suborg and senior official.""" self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1483,6 +1484,7 @@ def test_create_or_modify_portfolio(self): self.assertEqual(portfolio.senior_official, self.senior_official) def test_handle_portfolio_requests(self): + """Verify portfolio association with domain requests.""" self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True) self.domain_request.refresh_from_db() @@ -1490,6 +1492,7 @@ def test_handle_portfolio_requests(self): self.assertEqual(self.domain_request.portfolio.federal_agency, self.federal_agency) def test_handle_portfolio_domains(self): + """Check portfolio association with domain information.""" self.run_create_federal_portfolio("Test Federal Agency", parse_domains=True) self.domain_info.refresh_from_db() @@ -1497,6 +1500,7 @@ def test_handle_portfolio_domains(self): self.assertEqual(self.domain_info.portfolio.federal_agency, self.federal_agency) def test_handle_parse_both(self): + """Ensure correct parsing of both requests and domains.""" self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True, parse_domains=True) self.domain_request.refresh_from_db() @@ -1506,12 +1510,14 @@ def test_handle_parse_both(self): self.assertEqual(self.domain_request.portfolio, self.domain_info.portfolio) def test_command_error_no_parse_options(self): + """Verify error when no parse options are provided.""" with self.assertRaisesRegex( CommandError, "You must specify at least one of --parse_requests or --parse_domains." ): self.run_create_federal_portfolio("Test Federal Agency") def test_command_error_agency_not_found(self): + """Check error handling for non-existent agency.""" expected_message = ( "Cannot find the federal agency 'Non-existent Agency' in our database. " "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding." @@ -1520,6 +1526,7 @@ def test_command_error_agency_not_found(self): self.run_create_federal_portfolio("Non-existent Agency", parse_requests=True) def test_update_existing_portfolio(self): + """Test updating an existing portfolio.""" # Create an existing portfolio existing_portfolio = Portfolio.objects.create( federal_agency=self.federal_agency, From db16ba284b412f80d1cfc3503bc317809e6d15a6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:35:45 -0600 Subject: [PATCH 15/65] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 017854b914..d027628170 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -127,7 +127,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): - # Stored in variables due to linter wanting type information here + # Stored in variables due to linter wanting type information here. portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" if name is not None and name.lower() == portfolio_name.lower(): # You can use this to populate location information, when this occurs. @@ -138,7 +138,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) + new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) From 02bc0cba1a942c90cec953d499fcc31db67698c5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:49:43 -0600 Subject: [PATCH 16/65] Add suborg info to domains and requests --- .../commands/create_federal_portfolio.py | 38 ++++++++++++------- .../tests/test_management_scripts.py | 2 + 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d027628170..cad318b571 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -185,13 +185,18 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa if not domain_requests.exists(): message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - else: - for domain_request in domain_requests: - domain_request.portfolio = portfolio - - DomainRequest.objects.bulk_update(domain_requests, ["portfolio"]) - message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + return None + + # Get all suborg information and store it in a dict to avoid doing a db call + suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") + for domain_request in domain_requests: + domain_request.portfolio = portfolio + if domain_request.organization_name in suborgs: + domain_request.sub_organization = suborgs.get(domain_request.organization_name) + + DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) + message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): """ @@ -202,10 +207,15 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal if not domain_infos.exists(): message = "Portfolios not added to domains: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - else: - for domain_info in domain_infos: - domain_info.portfolio = portfolio - - DomainInformation.objects.bulk_update(domain_infos, ["portfolio"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + return None + + # Get all suborg information and store it in a dict to avoid doing a db call + suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") + for domain_info in domain_infos: + domain_info.portfolio = portfolio + if domain_info.organization_name in suborgs: + domain_info.sub_organization = suborgs.get(domain_info.organization_name) + + DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains" + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 29db8a4c59..cbdc2c0346 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1490,6 +1490,7 @@ def test_handle_portfolio_requests(self): self.domain_request.refresh_from_db() self.assertIsNotNone(self.domain_request.portfolio) self.assertEqual(self.domain_request.portfolio.federal_agency, self.federal_agency) + self.assertEqual(self.domain_request.sub_organization.name, "Testorg") def test_handle_portfolio_domains(self): """Check portfolio association with domain information.""" @@ -1498,6 +1499,7 @@ def test_handle_portfolio_domains(self): self.domain_info.refresh_from_db() self.assertIsNotNone(self.domain_info.portfolio) self.assertEqual(self.domain_info.portfolio.federal_agency, self.federal_agency) + self.assertEqual(self.domain_info.sub_organization.name, "Testorg") def test_handle_parse_both(self): """Ensure correct parsing of both requests and domains.""" From 8732af18d9bf94b16a487e959be1a1f7dae25d9e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 30 Aug 2024 09:55:29 -0600 Subject: [PATCH 17/65] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index cad318b571..ff97d9db16 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -186,7 +186,7 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa message = "Portfolios not added to domain requests: no valid records found" TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None - + # Get all suborg information and store it in a dict to avoid doing a db call suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: From 82cd91d923be346552e7db950f489f198e87d57d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:46:12 -0600 Subject: [PATCH 18/65] add portfolios view --- src/registrar/admin.py | 4 ++- src/registrar/models/user.py | 3 +++ .../django/admin/user_change_form.html | 26 +++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6b42cf96ba..640037847d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -962,7 +962,9 @@ def change_view(self, request, object_id, form_url="", extra_context=None): domain_ids = user_domain_roles.values_list("domain_id", flat=True) domains = Domain.objects.filter(id__in=domain_ids).exclude(state=Domain.State.DELETED) - extra_context = {"domain_requests": domain_requests, "domains": domains} + portfolio_ids = obj.get_portfolios().values_list("portfolio", flat=True) + portfolios = models.Portfolio.objects.filter(id__in=portfolio_ids) + extra_context = {"domain_requests": domain_requests, "domains": domains, "portfolios": portfolios} return super().change_view(request, object_id, form_url, extra_context) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index a7ea1e14ad..48bde5281f 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -244,6 +244,9 @@ def get_first_portfolio(self): if permission: return permission.portfolio return None + + def get_portfolios(self): + return self.portfolio_permissions.all() @classmethod def needs_identity_verification(cls, email, uuid): diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index 005d67aecd..c78fae6cb5 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -1,7 +1,33 @@ {% extends 'django/admin/email_clipboard_change_form.html' %} {% load i18n static %} +{% block field_sets %} + {% for fieldset in adminform %} + {% include "django/admin/includes/email_clipboard_fieldset.html" %} + {% endfor %} +{% endblock %} + {% block after_related_objects %} + {% if portfolios %} +
+

Portfolio information

+
+
+

Portfolios

+ +
+
+
+ {% endif %} +

Associated requests and domains

From c70e098f5401e3860de73232aec883e0e827bf22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:46:31 -0600 Subject: [PATCH 19/65] Update user_change_form.html --- src/registrar/templates/django/admin/user_change_form.html | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index c78fae6cb5..3a7ea5f928 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -1,12 +1,6 @@ {% extends 'django/admin/email_clipboard_change_form.html' %} {% load i18n static %} -{% block field_sets %} - {% for fieldset in adminform %} - {% include "django/admin/includes/email_clipboard_fieldset.html" %} - {% endfor %} -{% endblock %} - {% block after_related_objects %} {% if portfolios %}
From 1cb14f8d6a0ef03fbae3d4ea3b01244dba1f3489 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:48:26 -0600 Subject: [PATCH 20/65] Update user_change_form.html --- src/registrar/templates/django/admin/user_change_form.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/user_change_form.html b/src/registrar/templates/django/admin/user_change_form.html index 3a7ea5f928..b545bed23c 100644 --- a/src/registrar/templates/django/admin/user_change_form.html +++ b/src/registrar/templates/django/admin/user_change_form.html @@ -11,7 +11,7 @@

Portfolios

    {% for portfolio in portfolios %}
  • - + {{ portfolio }}
    • From c57405a918c673adc672b62236f67f27f13f3e22 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 3 Sep 2024 15:25:06 -0600 Subject: [PATCH 21/65] Update test_admin.py --- src/registrar/tests/test_admin.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a435c6a693..2e9122c386 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,6 +2,7 @@ from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite +from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from api.tests.common import less_console_noise_decorator from django.urls import reverse from registrar.admin import ( @@ -41,6 +42,7 @@ TransitionDomain, Portfolio, Suborganization, + UserPortfolioPermission, ) from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.senior_official import SeniorOfficial @@ -1223,6 +1225,25 @@ def test_analyst_cannot_see_selects_for_portfolio_role_and_permissions_in_user_f self.assertNotContains(response, "Portfolio roles:") self.assertNotContains(response, "Portfolio additional permissions:") + + @less_console_noise_decorator + def test_user_can_see_related_portfolios(self): + """Tests if a user can see the portfolios they are associated with on the user page""" + portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.user) + permission, _ = UserPortfolioPermission.objects.get_or_create( + user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] + ) + self.user.refresh_from_db() + self.client.force_login(self.user) + response = self.client.get( + "/admin/registrar/user/{}/change/".format(self.user.id), + follow=True, + ) + expected_href = reverse("admin:registrar_portfolio_change", args=[portfolio.pk]) + self.assertContains(response, expected_href) + self.assertContains(response, str(portfolio)) + permission.delete() + portfolio.delete() class AuditedAdminTest(TestCase): From 7a77cb8444343ae897f580dd65fc487bbebeb63d Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 12:37:01 -0500 Subject: [PATCH 22/65] initial stab at a new json formatter --- src/registrar/config/settings.py | 43 ++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 8 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 73aecad7a5..016dda4d9e 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -23,6 +23,8 @@ from pathlib import Path from typing import Final from botocore.config import Config +import json +from django.utils.log import ServerFormatter # # # ### # Setup code goes here # @@ -192,7 +194,7 @@ "registrar.registrar_middleware.CheckPortfolioMiddleware", ] -# application object used by Django’s built-in servers (e.g. `runserver`) +# application object used by Django's built-in servers (e.g. `runserver`) WSGI_APPLICATION = "registrar.config.wsgi.application" # endregion @@ -410,7 +412,7 @@ # and to interpret datetimes entered in forms TIME_ZONE = "UTC" -# enable Django’s translation system +# enable Django's translation system USE_I18N = True # enable localized formatting of numbers and dates @@ -445,6 +447,30 @@ # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") +class JsonFormatter(logging.Formatter): + def __init__(self): + super().__init__(datefmt="%d/%b/%Y %H:%M:%S") + + def format(self, record): + log_record = { + "timestamp": self.formatTime(record, self.datefmt), + "level": record.levelname, + "name": record.name, + "lineno": record.lineno, + "message": record.getMessage(), + } + return json.dumps(log_record) + +class JsonServerFormatter(ServerFormatter): + def format(self, record): + formatted_record = super().format(record) + log_entry = { + "server_time": record.server_time, + "level": record.levelname, + "message": formatted_record + } + return json.dumps(log_entry) + LOGGING = { "version": 1, # Don't import Django's existing loggers @@ -459,10 +485,11 @@ "simple": { "format": "%(levelname)s %(message)s", }, - "django.server": { - "()": "django.utils.log.ServerFormatter", - "format": "[{server_time}] {message}", - "style": "{", + "json.server": { + "()": JsonServerFormatter, + }, + "json": { + "()": JsonFormatter, }, }, # define where log messages will be sent; @@ -471,12 +498,12 @@ "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "verbose", + "formatter": "json", }, "django.server": { "level": "INFO", "class": "logging.StreamHandler", - "formatter": "django.server", + "formatter": "json.server", }, # No file logger is configured, # because containerized apps From e2fde1f96808f0c00899e483905e6da4a7ee4dbe Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 12:59:18 -0500 Subject: [PATCH 23/65] fix imports --- src/registrar/config/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 016dda4d9e..0b68a15747 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,6 +24,7 @@ from typing import Final from botocore.config import Config import json +import logging from django.utils.log import ServerFormatter # # # ### From afd941e27ffa7db39cf179255fb25edd5ec7b6c0 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:26:54 -0500 Subject: [PATCH 24/65] temp: add error message to test logs --- src/registrar/models/domain_request.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b80e063cde..9cee17b181 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,4 +1,5 @@ from __future__ import annotations +import traceback from typing import Union import logging from django.apps import apps @@ -644,6 +645,8 @@ def save(self, *args, **kwargs): self.sync_organization_type() self.sync_yes_no_form_fields() + logger.error(traceback.print_stack()) + if self._cached_status != self.status: self.last_status_update = timezone.now().date() From a61e8a16707466afee92af9ac35211f17fc162e2 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:42:31 -0500 Subject: [PATCH 25/65] more temp changes --- src/registrar/models/domain_request.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 9cee17b181..f6e215a271 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -644,8 +644,10 @@ def save(self, *args, **kwargs): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - - logger.error(traceback.print_stack()) + try: + raise ValueError("TEST TEST TEST") + except Exception as e: + logger.error(e) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From d9898e3af2643985e091f3d72904643749e09458 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:52:13 -0500 Subject: [PATCH 26/65] moar testing!!! --- src/registrar/models/domain_request.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f6e215a271..95a20d4cdb 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -645,9 +645,9 @@ def save(self, *args, **kwargs): self.sync_organization_type() self.sync_yes_no_form_fields() try: - raise ValueError("TEST TEST TEST") - except Exception as e: - logger.error(e) + raise Exception("TEST TEST TEST") + except Exception: + logger.error(traceback.format_exc) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From bdec43e2f349ac52dbac79ef0abeaf5ebe7ac5d5 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 4 Sep 2024 15:57:58 -0500 Subject: [PATCH 27/65] forgot parentheses :( --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 95a20d4cdb..af8d22b5ff 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -647,7 +647,7 @@ def save(self, *args, **kwargs): try: raise Exception("TEST TEST TEST") except Exception: - logger.error(traceback.format_exc) + logger.error(traceback.format_exc()) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From c043f44ef9a2f072eec36e535c140e1b0dd79ad3 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 17:59:54 -0400 Subject: [PATCH 28/65] refactoring code to meet ac reqs --- src/registrar/views/domain.py | 40 ++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 003f8dd0de..1fc6cb1efd 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -836,6 +836,23 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success exc_info=True, ) return None + + # Check to see if an invite has already been sent (NOTE: we do not want to create an invite just yet.) + try: + invite = DomainInvitation.objects.get(email=email, domain=self.object) + # that invitation already existed + if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: + messages.warning( + self.request, + f"{email} is already a manager for this domain.", + ) + else: + messages.warning( + self.request, + f"{email} has already been invited to this domain" + ) + except DomainInvitation.DoesNotExist: + logger.info("Domain Invitation Does Not Exist") try: send_templated_email( @@ -859,25 +876,14 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success else: if add_success: messages.success(self.request, f"{email} has been invited to this domain.") - + def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" - # Check to see if an invite has already been sent (NOTE: we do not want to create an invite just yet.) try: - invite = DomainInvitation.objects.get(email=email_address, domain=self.object) - # that invitation already existed - if invite is not None: - messages.warning( - self.request, - f"{email_address} has already been invited to this domain.", - ) - except DomainInvitation.DoesNotExist: - # Try to send the invitation. If it succeeds, add it to the DomainInvitation table. - try: - self._send_domain_invitation_email(email=email_address, requestor=requestor) - except EmailSendingError: + self._send_domain_invitation_email(email=email_address, requestor=requestor, add_success=False) + except EmailSendingError: messages.warning(self.request, "Could not send email invitation.") - else: + else: # (NOTE: only create a domainInvitation if the e-mail sends correctly) DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) return redirect(self.get_success_url()) @@ -921,8 +927,8 @@ def form_valid(self, form): except IntegrityError: # User already has the desired role! Do nothing?? pass - - messages.success(self.request, f"Added user {requested_email}.") + else: + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 8da02de034de887690f4bf38e2aa5809de501b7e Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:18:54 -0400 Subject: [PATCH 29/65] removed else from message success --- src/registrar/views/domain.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 1fc6cb1efd..e483c3f857 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -839,6 +839,7 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success # Check to see if an invite has already been sent (NOTE: we do not want to create an invite just yet.) try: + add_success=False invite = DomainInvitation.objects.get(email=email, domain=self.object) # that invitation already existed if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: @@ -880,7 +881,7 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" try: - self._send_domain_invitation_email(email=email_address, requestor=requestor, add_success=False) + self._send_domain_invitation_email(email=email_address, requestor=requestor) except EmailSendingError: messages.warning(self.request, "Could not send email invitation.") else: @@ -927,8 +928,8 @@ def form_valid(self, form): except IntegrityError: # User already has the desired role! Do nothing?? pass - else: - messages.success(self.request, f"Added user {requested_email}.") + + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 0e7b5ae8cba88e3af95d13304c929146c2495c9e Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:19:48 -0400 Subject: [PATCH 30/65] removed line --- src/registrar/views/domain.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index e483c3f857..fcac53f290 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -927,8 +927,7 @@ def form_valid(self, form): ) except IntegrityError: # User already has the desired role! Do nothing?? - pass - + pass messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 914fbdc04b53310cf8abc0b23d8cfb08d734417d Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:24:07 -0400 Subject: [PATCH 31/65] added comments --- src/registrar/views/domain.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index fcac53f290..0cdfa0c774 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -837,17 +837,19 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success ) return None - # Check to see if an invite has already been sent (NOTE: we do not want to create an invite just yet.) + # Check to see if an invite has already been sent try: - add_success=False invite = DomainInvitation.objects.get(email=email, domain=self.object) - # that invitation already existed + # check if the invite has already been accepted if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: + add_success=False messages.warning( self.request, f"{email} is already a manager for this domain.", ) else: + add_success=False + #else if it has been sent but not accepted messages.warning( self.request, f"{email} has already been invited to this domain" From 236aac02b004091cba434d1bdaea0f98ca86f99b Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:27:32 -0400 Subject: [PATCH 32/65] an error occured --- src/registrar/views/domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 0cdfa0c774..e0e59dd590 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -854,8 +854,8 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success self.request, f"{email} has already been invited to this domain" ) - except DomainInvitation.DoesNotExist: - logger.info("Domain Invitation Does Not Exist") + except Exception: + logger.error("An error occured") try: send_templated_email( From 0d00bd467d6c9638f47df6b2d06ece6e537a7b70 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:27:57 -0400 Subject: [PATCH 33/65] removed spacing --- src/registrar/views/domain.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index e0e59dd590..eef2f0451e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -931,7 +931,6 @@ def form_valid(self, form): # User already has the desired role! Do nothing?? pass messages.success(self.request, f"Added user {requested_email}.") - return redirect(self.get_success_url()) From ef30e06b89ffd6b6aa78408b58faecfa51029032 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 18:32:07 -0400 Subject: [PATCH 34/65] added back the sucess message under else --- src/registrar/views/domain.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index eef2f0451e..b3b27f5816 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -928,9 +928,9 @@ def form_valid(self, form): role=UserDomainRole.Roles.MANAGER, ) except IntegrityError: - # User already has the desired role! Do nothing?? - pass - messages.success(self.request, f"Added user {requested_email}.") + messages.warning(self.request, f"{requested_email} already added to this domain") + else: + messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 68b8a6086b4ae97b5bff59bd03030ecfb8ece473 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 4 Sep 2024 19:53:44 -0400 Subject: [PATCH 35/65] formated changes --- src/registrar/views/domain.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index b3b27f5816..a672f37ef4 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -836,24 +836,21 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success exc_info=True, ) return None - + # Check to see if an invite has already been sent try: invite = DomainInvitation.objects.get(email=email, domain=self.object) # check if the invite has already been accepted if invite.status == DomainInvitation.DomainInvitationStatus.RETRIEVED: - add_success=False + add_success = False messages.warning( self.request, f"{email} is already a manager for this domain.", ) else: - add_success=False - #else if it has been sent but not accepted - messages.warning( - self.request, - f"{email} has already been invited to this domain" - ) + add_success = False + # else if it has been sent but not accepted + messages.warning(self.request, f"{email} has already been invited to this domain") except Exception: logger.error("An error occured") @@ -879,16 +876,16 @@ def _send_domain_invitation_email(self, email: str, requestor: User, add_success else: if add_success: messages.success(self.request, f"{email} has been invited to this domain.") - + def _make_invitation(self, email_address: str, requestor: User): """Make a Domain invitation for this email and redirect with a message.""" try: - self._send_domain_invitation_email(email=email_address, requestor=requestor) + self._send_domain_invitation_email(email=email_address, requestor=requestor) except EmailSendingError: - messages.warning(self.request, "Could not send email invitation.") + messages.warning(self.request, "Could not send email invitation.") else: - # (NOTE: only create a domainInvitation if the e-mail sends correctly) - DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) + # (NOTE: only create a domainInvitation if the e-mail sends correctly) + DomainInvitation.objects.get_or_create(email=email_address, domain=self.object) return redirect(self.get_success_url()) def form_valid(self, form): @@ -928,7 +925,7 @@ def form_valid(self, form): role=UserDomainRole.Roles.MANAGER, ) except IntegrityError: - messages.warning(self.request, f"{requested_email} already added to this domain") + messages.warning(self.request, f"{requested_email} already added to this domain") else: messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 9d4ab22dc4bed49b0e3b226ff9e69e45ff18c8b7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 5 Sep 2024 08:53:37 -0600 Subject: [PATCH 36/65] fix unit test --- src/registrar/models/user.py | 2 +- src/registrar/tests/test_admin.py | 17 +++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 48bde5281f..0af6c357ba 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -244,7 +244,7 @@ def get_first_portfolio(self): if permission: return permission.portfolio return None - + def get_portfolios(self): return self.portfolio_permissions.all() diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 2e9122c386..3e4b8fb459 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -3,6 +3,7 @@ from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices +from django_webtest import WebTest # type: ignore from api.tests.common import less_console_noise_decorator from django.urls import reverse from registrar.admin import ( @@ -972,7 +973,7 @@ def test_get_filters(self): ) -class TestMyUserAdmin(MockDbForSharedTests): +class TestMyUserAdmin(MockDbForSharedTests, WebTest): """Tests for the MyUserAdmin class as super or staff user Notes: @@ -992,6 +993,7 @@ def setUpClass(cls): def setUp(self): super().setUp() + self.app.set_user(self.superuser.username) self.client = Client(HTTP_HOST="localhost:8080") def tearDown(self): @@ -1225,20 +1227,15 @@ def test_analyst_cannot_see_selects_for_portfolio_role_and_permissions_in_user_f self.assertNotContains(response, "Portfolio roles:") self.assertNotContains(response, "Portfolio additional permissions:") - + @less_console_noise_decorator def test_user_can_see_related_portfolios(self): """Tests if a user can see the portfolios they are associated with on the user page""" - portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.user) + portfolio, _ = Portfolio.objects.get_or_create(organization_name="test", creator=self.superuser) permission, _ = UserPortfolioPermission.objects.get_or_create( - user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] - ) - self.user.refresh_from_db() - self.client.force_login(self.user) - response = self.client.get( - "/admin/registrar/user/{}/change/".format(self.user.id), - follow=True, + user=self.superuser, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN] ) + response = self.app.get(reverse("admin:registrar_user_change", args=[self.superuser.pk])) expected_href = reverse("admin:registrar_portfolio_change", args=[portfolio.pk]) self.assertContains(response, expected_href) self.assertContains(response, str(portfolio)) From 23d471f11c96b519e4687ce4a44b50412cda6ce5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 6 Sep 2024 10:08:50 -0600 Subject: [PATCH 37/65] Add --both --- .../commands/create_federal_portfolio.py | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index ff97d9db16..2dbe152ab9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -33,29 +33,41 @@ def add_arguments(self, parser): action=argparse.BooleanOptionalAction, help="Adds portfolio to DomainInformation", ) + parser.add_argument( + "--both", + action=argparse.BooleanOptionalAction, + help="Adds portfolio to both requests and domains", + ) def handle(self, agency_name, **options): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") + both = options.get("both") - if not parse_requests and not parse_domains: - raise CommandError("You must specify at least one of --parse_requests or --parse_domains.") + if not both: + if not parse_requests and not parse_domains: + raise CommandError("You must specify at least one of --parse_requests or --parse_domains.") + else: + if parse_requests or parse_domains: + raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first() if not federal_agency: - raise ValueError( + message = ( f"Cannot find the federal agency '{agency_name}' in our database. " "The value you enter for `agency_name` must be " "prepopulated in the FederalAgency table before proceeding." ) + TerminalHelper.colorful_logger(logger.error, TerminalColors.FAIL, message) + return None portfolio = self.create_or_modify_portfolio(federal_agency) self.create_suborganizations(portfolio, federal_agency) - if parse_requests: + if parse_requests or both: self.handle_portfolio_requests(portfolio, federal_agency) - if parse_domains: + if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) def create_or_modify_portfolio(self, federal_agency): @@ -115,7 +127,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA if not org_names: TerminalHelper.colorful_logger( - logger.warning, TerminalColors.YELLOW, f"No suborganizations found for {federal_agency}" + logger.warning, TerminalColors.YELLOW, f"No suborganizations found for '{federal_agency}'" ) return From 44e363da0439615d8b86c33e12a373f2c202212d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 6 Sep 2024 10:14:06 -0600 Subject: [PATCH 38/65] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 2dbe152ab9..1a4650c384 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -53,13 +53,11 @@ def handle(self, agency_name, **options): federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first() if not federal_agency: - message = ( + raise ValueError( f"Cannot find the federal agency '{agency_name}' in our database. " "The value you enter for `agency_name` must be " "prepopulated in the FederalAgency table before proceeding." ) - TerminalHelper.colorful_logger(logger.error, TerminalColors.FAIL, message) - return None portfolio = self.create_or_modify_portfolio(federal_agency) self.create_suborganizations(portfolio, federal_agency) From 39c881d7a5de515834e88066e4c63a9fccd53bca Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Mon, 9 Sep 2024 13:19:38 -0400 Subject: [PATCH 39/65] account for different superuser and analyst markup --- src/registrar/assets/js/get-gov-admin.js | 10 ++++++++-- .../django/admin/includes/detail_table_fieldset.html | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 27ff1470b6..7ff02ba1f5 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -748,7 +748,10 @@ function initializeWidgetOnList(list, parentId) { //------ Requested Domains const requestedDomainElement = document.getElementById('id_requested_domain'); - const requestedDomain = requestedDomainElement.options[requestedDomainElement.selectedIndex].text; + // We have to account for different superuser and analyst markups + const requestedDomain = requestedDomainElement.options + ? requestedDomainElement.options[requestedDomainElement.selectedIndex].text + : requestedDomainElement.text; //------ Submitter // Function to extract text by ID and handle missing elements @@ -762,7 +765,10 @@ function initializeWidgetOnList(list, parentId) { // Extract the submitter name, title, email, and phone number const submitterDiv = document.querySelector('.form-row.field-submitter'); const submitterNameElement = document.getElementById('id_submitter'); - const submitterName = submitterNameElement.options[submitterNameElement.selectedIndex].text; + // We have to account for different superuser and analyst markups + const submitterName = submitterNameElement + ? submitterNameElement.options[submitterNameElement.selectedIndex].text + : submitterDiv.querySelector('a').text; const submitterTitle = extractTextById('contact_info_title', submitterDiv); const submitterEmail = extractTextById('contact_info_email', submitterDiv); const submitterPhone = extractTextById('contact_info_phone', submitterDiv); diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 5e10571398..db156b033f 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -107,7 +107,7 @@ {% endif %} {% elif field.field.name == "requested_domain" %} {% with current_path=request.get_full_path %} - {{ original.requested_domain }} + {{ original.requested_domain }} {% endwith%} {% elif field.field.name == "current_websites" %} {% comment %} From 3955d5648fe52f0a2939b4cc4930e63bd206b21f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:05:13 -0600 Subject: [PATCH 40/65] Update user.py --- src/registrar/models/user.py | 73 ++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 0af6c357ba..4e789ff0c3 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -224,14 +224,87 @@ def has_domains_portfolio_permission(self, portfolio): ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MANAGED_DOMAINS) def has_domain_requests_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_requests_flag = flag_is_active(request, "organization_requests") + if not has_organization_requests_flag: + return False + # END return self._has_portfolio_permission( portfolio, UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS ) or self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_CREATED_REQUESTS) + def has_view_members_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + # END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_MEMBERS) + + def has_edit_members_portfolio_permission(self, portfolio): + # BEGIN + # Note code below is to add organization_request feature + request = HttpRequest() + request.user = self + has_organization_members_flag = flag_is_active(request, "organization_members") + if not has_organization_members_flag: + return False + # END + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_MEMBERS) + def has_view_all_domains_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + def has_edit_requests(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) + + def portfolio_role_summary(self, portfolio): + """Returns a list of roles based on the user's permissions.""" + roles = [] + + # Define the conditions and their corresponding roles + conditions_roles = [ + (self.has_edit_suborganization(portfolio), ["Admin"]), + ( + self.has_view_all_domains_permission(portfolio) + and self.has_domain_requests_portfolio_permission(portfolio) + and self.has_edit_requests(portfolio), + ["View-only admin", "Domain requestor"], + ), + ( + self.has_view_all_domains_permission(portfolio) + and self.has_domain_requests_portfolio_permission(portfolio), + ["View-only admin"], + ), + ( + self.has_base_portfolio_permission(portfolio) + and self.has_edit_requests(portfolio) + and self.has_domains_portfolio_permission(portfolio), + ["Domain requestor", "Domain manager"], + ), + (self.has_base_portfolio_permission(portfolio) and self.has_edit_requests(portfolio), ["Domain requestor"]), + ( + self.has_base_portfolio_permission(portfolio) and self.has_domains_portfolio_permission(portfolio), + ["Domain manager"], + ), + (self.has_base_portfolio_permission(portfolio), ["Member"]), + ] + + # Evaluate conditions and add roles + for condition, role_list in conditions_roles: + if condition: + roles.extend(role_list) + break + + return roles + # Field specific permission checks def has_view_suborganization(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) From c5c3043eaaaebe0129faae4cab6a559636ca6657 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:05:37 -0600 Subject: [PATCH 41/65] Update user.py --- src/registrar/models/user.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 4e789ff0c3..45ffbadb77 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -6,7 +6,7 @@ from django.http import HttpRequest from registrar.models import DomainInformation, UserDomainRole -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices from .domain_invitation import DomainInvitation from .portfolio_invitation import PortfolioInvitation @@ -64,32 +64,6 @@ class VerificationTypeChoices(models.TextChoices): # after they login. FIXTURE_USER = "fixture_user", "Created by fixtures" - PORTFOLIO_ROLE_PERMISSIONS = { - UserPortfolioRoleChoices.ORGANIZATION_ADMIN: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.EDIT_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.EDIT_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - UserPortfolioPermissionChoices.EDIT_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_ADMIN_READ_ONLY: [ - UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS, - UserPortfolioPermissionChoices.VIEW_MEMBER, - UserPortfolioPermissionChoices.VIEW_ALL_REQUESTS, - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - # Domain: field specific permissions - UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION, - ], - UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [ - UserPortfolioPermissionChoices.VIEW_PORTFOLIO, - ], - } - # #### Constants for choice fields #### RESTRICTED = "restricted" STATUS_CHOICES = ((RESTRICTED, RESTRICTED),) From 9e7fa80bd0c786b9c69ead25bd84e6115bff9999 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 9 Sep 2024 13:07:01 -0600 Subject: [PATCH 42/65] Update user.py --- src/registrar/models/user.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py index 45ffbadb77..901ab62af1 100644 --- a/src/registrar/models/user.py +++ b/src/registrar/models/user.py @@ -236,6 +236,19 @@ def has_view_all_domains_permission(self, portfolio): """Determines if the current user can view all available domains in a given portfolio""" return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_ALL_DOMAINS) + # Field specific permission checks + def has_view_suborganization(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) + + def has_edit_suborganization(self, portfolio): + return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) + + def get_first_portfolio(self): + permission = self.portfolio_permissions.first() + if permission: + return permission.portfolio + return None + def has_edit_requests(self, portfolio): return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS) @@ -279,19 +292,6 @@ def portfolio_role_summary(self, portfolio): return roles - # Field specific permission checks - def has_view_suborganization(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION) - - def has_edit_suborganization(self, portfolio): - return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION) - - def get_first_portfolio(self): - permission = self.portfolio_permissions.first() - if permission: - return permission.portfolio - return None - def get_portfolios(self): return self.portfolio_permissions.all() From 99cfa0ee971b10daf618e8fc293f68d8b03aeffc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 10 Sep 2024 08:38:56 -0600 Subject: [PATCH 43/65] linting --- src/registrar/tests/test_admin.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 5bdf3560f2..83114b3b34 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2,7 +2,6 @@ from django.utils import timezone from django.test import TestCase, RequestFactory, Client from django.contrib.admin.sites import AdminSite -from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices from django_webtest import WebTest # type: ignore from api.tests.common import less_console_noise_decorator from django.urls import reverse @@ -44,13 +43,11 @@ Portfolio, Suborganization, UserPortfolioPermission, + UserDomainRole, + SeniorOfficial, + PortfolioInvitation, + VerifiedByStaff, ) -from registrar.models.portfolio_invitation import PortfolioInvitation -from registrar.models.senior_official import SeniorOfficial -from registrar.models.user_domain_role import UserDomainRole -from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices -from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( MockDbForSharedTests, AuditedAdminMockData, @@ -63,10 +60,11 @@ multiple_unalphabetical_domain_objects, GenericTestHelper, ) +from registrar.models.utility.portfolio_helper import UserPortfolioPermissionChoices, UserPortfolioRoleChoices from django.contrib.sessions.backends.db import SessionStore from django.contrib.auth import get_user_model from unittest.mock import ANY, patch, Mock -from django_webtest import WebTest # type: ignore + import logging From aa01036e0fd0b9d25a5492b8756eea1077d591fb Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 09:49:19 -0400 Subject: [PATCH 44/65] updated text --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a672f37ef4..0c240f8820 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -925,7 +925,7 @@ def form_valid(self, form): role=UserDomainRole.Roles.MANAGER, ) except IntegrityError: - messages.warning(self.request, f"{requested_email} already added to this domain") + messages.warning(self.request, f"{requested_email} already a manager for this domain") else: messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 31b7daf5641fcdc4e0993803e3595cb86b4e2b4a Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 09:50:47 -0400 Subject: [PATCH 45/65] added an is --- src/registrar/views/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 0c240f8820..36091d77b4 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -925,7 +925,7 @@ def form_valid(self, form): role=UserDomainRole.Roles.MANAGER, ) except IntegrityError: - messages.warning(self.request, f"{requested_email} already a manager for this domain") + messages.warning(self.request, f"{requested_email} is already a manager for this domain") else: messages.success(self.request, f"Added user {requested_email}.") return redirect(self.get_success_url()) From 5651b57d1ea6de70324f86cbcd471ef7bb575b64 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 10:19:26 -0400 Subject: [PATCH 46/65] updated upload-artifact --- .github/workflows/security-check.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index dd75d5c98c..aea700613a 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -44,7 +44,7 @@ jobs: id: check uses: ./.github/actions/django-security-check - name: Upload output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: security-check-output path: ./src/output.txt From 60dedcde4e9f05441f0bdb6f549c1b3c608622e7 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 10:19:48 -0400 Subject: [PATCH 47/65] updated upload-artifact --- .github/actions/django-security-check/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/django-security-check/README.md b/.github/actions/django-security-check/README.md index 4eddcf74c0..94f02a97cc 100644 --- a/.github/actions/django-security-check/README.md +++ b/.github/actions/django-security-check/README.md @@ -38,7 +38,7 @@ jobs: id: check uses: victoriadrake/django-security-check@master - name: Upload output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: security-check-output path: output.txt From e921334a26b99455c3929cb06e646685726920e6 Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 10:25:53 -0400 Subject: [PATCH 48/65] updated actions/upload-artifact to version4 due to failing checks --- .github/actions/django-security-check/README.md | 2 +- .github/workflows/security-check.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/django-security-check/README.md b/.github/actions/django-security-check/README.md index 4eddcf74c0..94f02a97cc 100644 --- a/.github/actions/django-security-check/README.md +++ b/.github/actions/django-security-check/README.md @@ -38,7 +38,7 @@ jobs: id: check uses: victoriadrake/django-security-check@master - name: Upload output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: security-check-output path: output.txt diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index dd75d5c98c..aea700613a 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -44,7 +44,7 @@ jobs: id: check uses: ./.github/actions/django-security-check - name: Upload output - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v4 with: name: security-check-output path: ./src/output.txt From 7eaba2c56ba4229bc634b46a60a1a34ea0267d91 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 11 Sep 2024 08:45:54 -0600 Subject: [PATCH 49/65] Update fixtures_users.py --- src/registrar/fixtures_users.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/fixtures_users.py b/src/registrar/fixtures_users.py index 1b8eda9aba..7fbf41223d 100644 --- a/src/registrar/fixtures_users.py +++ b/src/registrar/fixtures_users.py @@ -56,6 +56,7 @@ class UserFixture: "username": "8f8e7293-17f7-4716-889b-1990241cbd39", "first_name": "Katherine", "last_name": "Osos", + "email": "kosos@truss.works", }, { "username": "70488e0a-e937-4894-a28c-16f5949effd4", @@ -171,7 +172,7 @@ class UserFixture: "username": "91a9b97c-bd0a-458d-9823-babfde7ebf44", "first_name": "Katherine-Analyst", "last_name": "Osos-Analyst", - "email": "kosos@truss.works", + "email": "kosos+1@truss.works", }, { "username": "2cc0cde8-8313-4a50-99d8-5882e71443e8", From 27f21577650821fe90be10df6c07a964b6b4fede Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 11 Sep 2024 09:05:44 -0600 Subject: [PATCH 50/65] Add more logging detail --- docs/operations/data_migration.md | 9 +++--- .../commands/create_federal_portfolio.py | 30 ++++++++++++------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 6c42cb727e..e54c35d28b 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -885,20 +885,21 @@ Example: `cf ssh getgov-za` [Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice. #### Step 5: Running the script -```./manage.py create_federal_portfolio "{federal_agency_name}" --parse_requests --parse_domains``` +```./manage.py create_federal_portfolio "{federal_agency_name}" --both``` Example: `./manage.py create_federal_portfolio "AMTRAK" --parse_requests --parse_domains` ### Running locally #### Step 1: Running the script -```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --parse_requests --parse_domains``` +```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --both``` ##### Parameters | | Parameter | Description | |:-:|:-------------------------- |:-------------------------------------------------------------------------------------------| | 1 | **federal_agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | -| 2 | **parse_requests** | Optional. If True, then the created portfolio is added to all related DomainRequests. | -| 3 | **parse_domains** | Optional. If True, then the created portfolio is added to all related Domains. | +| 2 | **both** | Optional. If True, runs parse_requests and parse_domains | +| 3 | **parse_requests** | Optional. If True, then the created portfolio is added to all related DomainRequests. | +| 4 | **parse_domains** | Optional. If True, then the created portfolio is added to all related Domains. | Note: While you can specify both at the same time, you must specify either --parse_requests or --parse_domains. You cannot run the script without defining one or the other. diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1a4650c384..01cf56fb53 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -91,7 +91,7 @@ def create_or_modify_portfolio(self, federal_agency): else: proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - info_to_inspect=f""" + prompt_message=f""" The given portfolio '{federal_agency.agency}' already exists in our DB. If you cancel, the rest of the script will still execute but this record will not update. """, @@ -124,9 +124,12 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA org_names = set(valid_agencies.values_list("organization_name", flat=True)) if not org_names: - TerminalHelper.colorful_logger( - logger.warning, TerminalColors.YELLOW, f"No suborganizations found for '{federal_agency}'" + message = ( + "Could not add any suborganizations." + f"\nNo suborganizations were found for '{federal_agency}' when filtering on this name, " + "and excluding null organization_name records." ) + TerminalHelper.colorful_logger(logger.warning, TerminalColors.FAIL, message) return # Check if we need to update any existing suborgs first. This step is optional. @@ -144,7 +147,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA # However, this isn't needed for now so we can skip it. message = ( f"Skipping suborganization create on record '{name}'. " - f"The federal agency name is the same as the portfolio name." + "The federal agency name is the same as the portfolio name." ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: @@ -165,7 +168,7 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): """ proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - info_to_inspect=f"""Some suborganizations already exist in our DB. + prompt_message=f"""Some suborganizations already exist in our DB. If you cancel, the rest of the script will still execute but these records will not update. ==Proposed Changes== @@ -178,7 +181,7 @@ def _update_existing_suborganizations(self, portfolio, orgs_to_update): org.portfolio = portfolio Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"]) - message = f"Updated {len(orgs_to_update)} suborganizations" + message = f"Updated {len(orgs_to_update)} suborganizations." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): @@ -192,8 +195,12 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa DomainRequest.DomainRequestStatus.REJECTED, ] domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) - if not domain_requests.exists(): - message = "Portfolios not added to domain requests: no valid records found" + if domain_requests.exists(): + message = f""" + Portfolios not added to domain requests: no valid records found. + This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + Excluded statuses: STARTED, INELIGIBLE, REJECTED. + """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None @@ -205,7 +212,7 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.sub_organization = suborgs.get(domain_request.organization_name) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) - message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests" + message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): @@ -215,7 +222,10 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal """ domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) if not domain_infos.exists(): - message = "Portfolios not added to domains: no valid records found" + message = f""" + Portfolios not added to domains: no valid records found. + This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + """ TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) return None From 97c7826455d5812bbf2f0249c6b1d4df36a57699 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 11 Sep 2024 09:13:00 -0600 Subject: [PATCH 51/65] Update src/registrar/management/commands/create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 01cf56fb53..b98bb0c840 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -195,7 +195,7 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa DomainRequest.DomainRequestStatus.REJECTED, ] domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) - if domain_requests.exists(): + if not domain_requests.exists(): message = f""" Portfolios not added to domain requests: no valid records found. This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. From eb3cb1e3d821d7e181c7b32e8784c3715101a633 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:34:32 -0500 Subject: [PATCH 52/65] Don't use json locally, plus review changes --- src/registrar/config/settings.py | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 0b68a15747..886f2df500 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -448,21 +448,8 @@ # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") -class JsonFormatter(logging.Formatter): - def __init__(self): - super().__init__(datefmt="%d/%b/%Y %H:%M:%S") - - def format(self, record): - log_record = { - "timestamp": self.formatTime(record, self.datefmt), - "level": record.levelname, - "name": record.name, - "lineno": record.lineno, - "message": record.getMessage(), - } - return json.dumps(log_record) - class JsonServerFormatter(ServerFormatter): + """Formats logs into JSON for easier and more accurate processing.""" def format(self, record): formatted_record = super().format(record) log_entry = { @@ -489,9 +476,6 @@ def format(self, record): "json.server": { "()": JsonServerFormatter, }, - "json": { - "()": JsonFormatter, - }, }, # define where log messages will be sent; # each logger can have one or more handlers @@ -499,7 +483,7 @@ def format(self, record): "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "json", + "formatter": "verbose", }, "django.server": { "level": "INFO", From ed4eeb469f4f7f8c3d8e9b324cb10ab147a9b673 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:42:26 -0500 Subject: [PATCH 53/65] linter fixes --- src/registrar/config/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 886f2df500..75881ab3fd 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,7 +24,6 @@ from typing import Final from botocore.config import Config import json -import logging from django.utils.log import ServerFormatter # # # ### @@ -448,6 +447,7 @@ # logger.error("Can't do this important task. Something is very wrong.") # logger.critical("Going to crash now.") + class JsonServerFormatter(ServerFormatter): """Formats logs into JSON for easier and more accurate processing.""" def format(self, record): @@ -459,6 +459,7 @@ def format(self, record): } return json.dumps(log_entry) + LOGGING = { "version": 1, # Don't import Django's existing loggers From 0488b452735bf2de99d9581e9fc1dbceccff17cf Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:49:27 -0500 Subject: [PATCH 54/65] remove test log --- src/registrar/models/domain_request.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index af8d22b5ff..b4a9881651 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -644,10 +644,6 @@ def save(self, *args, **kwargs): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - try: - raise Exception("TEST TEST TEST") - except Exception: - logger.error(traceback.format_exc()) if self._cached_status != self.status: self.last_status_update = timezone.now().date() From 635d8480f17a4380abfb2bd3ebc125c90bbe0565 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 10:50:10 -0500 Subject: [PATCH 55/65] remove unused import --- src/registrar/models/domain_request.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b4a9881651..b80e063cde 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1,5 +1,4 @@ from __future__ import annotations -import traceback from typing import Union import logging from django.apps import apps From 180a0240e52d75e359854b2da5d7d3d64dccdd49 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 13:09:48 -0500 Subject: [PATCH 56/65] conditionally set log format --- src/registrar/config/settings.py | 39 ++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 75881ab3fd..40d4f69331 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -24,6 +24,7 @@ from typing import Final from botocore.config import Config import json +import logging from django.utils.log import ServerFormatter # # # ### @@ -59,7 +60,7 @@ env_debug = env.bool("DJANGO_DEBUG", default=False) env_is_production = env.bool("IS_PRODUCTION", default=False) env_log_level = env.str("DJANGO_LOG_LEVEL", "DEBUG") -env_base_url = env.str("DJANGO_BASE_URL") +env_base_url: str = env.str("DJANGO_BASE_URL") env_getgov_public_site_url = env.str("GETGOV_PUBLIC_SITE_URL", "") env_oidc_active_provider = env.str("OIDC_ACTIVE_PROVIDER", "identity sandbox") @@ -448,8 +449,24 @@ # logger.critical("Going to crash now.") +class JsonFormatter(logging.Formatter): + """Formats logs into JSON for better parsing""" + def __init__(self): + super().__init__(datefmt="%d/%b/%Y %H:%M:%S") + + def format(self, record): + log_record = { + "timestamp": self.formatTime(record, self.datefmt), + "level": record.levelname, + "name": record.name, + "lineno": record.lineno, + "message": record.getMessage(), + } + return json.dumps(log_record) + + class JsonServerFormatter(ServerFormatter): - """Formats logs into JSON for easier and more accurate processing.""" + """Formats server logs into JSON for better parsing""" def format(self, record): formatted_record = super().format(record) log_entry = { @@ -459,6 +476,12 @@ def format(self, record): } return json.dumps(log_entry) +# default to json formatted logs +server_formatter, console_formatter = 'json.server', 'json' + +# don't use json format locally, it makes logs hard to read in console +if "localhost" in env_base_url: + server_formatter, console_formatter = 'django.server', 'verbose' LOGGING = { "version": 1, @@ -474,9 +497,17 @@ def format(self, record): "simple": { "format": "%(levelname)s %(message)s", }, + "django.server": { + "()": "django.utils.log.ServerFormatter", + "format": "[{server_time}] {message}", + "style": "{", + }, "json.server": { "()": JsonServerFormatter, }, + "json": { + "()": JsonFormatter, + }, }, # define where log messages will be sent; # each logger can have one or more handlers @@ -484,12 +515,12 @@ def format(self, record): "console": { "level": env_log_level, "class": "logging.StreamHandler", - "formatter": "verbose", + "formatter": console_formatter, }, "django.server": { "level": "INFO", "class": "logging.StreamHandler", - "formatter": "json.server", + "formatter": server_formatter, }, # No file logger is configured, # because containerized apps From d33f9ae8e2c7a966a7d61230387136eb85352f37 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Wed, 11 Sep 2024 13:34:19 -0500 Subject: [PATCH 57/65] linter errors --- src/registrar/config/settings.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 40d4f69331..b2a65fe237 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -451,6 +451,7 @@ class JsonFormatter(logging.Formatter): """Formats logs into JSON for better parsing""" + def __init__(self): super().__init__(datefmt="%d/%b/%Y %H:%M:%S") @@ -467,21 +468,19 @@ def format(self, record): class JsonServerFormatter(ServerFormatter): """Formats server logs into JSON for better parsing""" + def format(self, record): formatted_record = super().format(record) - log_entry = { - "server_time": record.server_time, - "level": record.levelname, - "message": formatted_record - } + log_entry = {"server_time": record.server_time, "level": record.levelname, "message": formatted_record} return json.dumps(log_entry) + # default to json formatted logs -server_formatter, console_formatter = 'json.server', 'json' +server_formatter, console_formatter = "json.server", "json" # don't use json format locally, it makes logs hard to read in console if "localhost" in env_base_url: - server_formatter, console_formatter = 'django.server', 'verbose' + server_formatter, console_formatter = "django.server", "verbose" LOGGING = { "version": 1, From 2069ab0d22db29f0502a1b34f87415ea5865eefa Mon Sep 17 00:00:00 2001 From: asaki222 Date: Wed, 11 Sep 2024 14:34:55 -0400 Subject: [PATCH 58/65] reverted the artifact change --- .github/actions/django-security-check/README.md | 2 +- .github/workflows/security-check.yaml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/django-security-check/README.md b/.github/actions/django-security-check/README.md index 94f02a97cc..4eddcf74c0 100644 --- a/.github/actions/django-security-check/README.md +++ b/.github/actions/django-security-check/README.md @@ -38,7 +38,7 @@ jobs: id: check uses: victoriadrake/django-security-check@master - name: Upload output - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v2 with: name: security-check-output path: output.txt diff --git a/.github/workflows/security-check.yaml b/.github/workflows/security-check.yaml index aea700613a..dd75d5c98c 100644 --- a/.github/workflows/security-check.yaml +++ b/.github/workflows/security-check.yaml @@ -44,7 +44,7 @@ jobs: id: check uses: ./.github/actions/django-security-check - name: Upload output - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v2 with: name: security-check-output path: ./src/output.txt From f2c48974197a8c449feef8fb31850e9d42cf7b30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 12 Sep 2024 08:23:10 -0600 Subject: [PATCH 59/65] doc update --- docs/operations/data_migration.md | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index e54c35d28b..9bde4b6950 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -863,10 +863,17 @@ Example: `cf ssh getgov-za` ## Create federal portfolio This script takes the name of a `FederalAgency` (like 'AMTRAK') and does the following: -1. Creates the portfolio record based off of data on the federal agency object itself -2. Creates suborganizations from existing DomainInformation records -3. Associates the SeniorOfficial record (if it exists) -4. Adds this portfolio to DomainInformation / DomainRequests or both +1. Creates the portfolio record based off of data on the federal agency object itself. +2. Creates suborganizations from existing DomainInformation records. +3. Associates the SeniorOfficial record (if it exists). +4. Adds this portfolio to DomainInformation / DomainRequests or both. + +Errors: +1. ValueError: Federal agency not found in database. +2. Logged Error: No suborganizations found for agency. +3. Logged Warning: No new suborganizations to add. +4. Logged Warning: No valid DomainRequest records to update. +5. Logged Warning: No valid DomainInformation records to update. ### Running on sandboxes @@ -887,7 +894,7 @@ Example: `cf ssh getgov-za` #### Step 5: Running the script ```./manage.py create_federal_portfolio "{federal_agency_name}" --both``` -Example: `./manage.py create_federal_portfolio "AMTRAK" --parse_requests --parse_domains` +Example (only requests): `./manage.py create_federal_portfolio "AMTRAK" --parse_requests` ### Running locally @@ -898,8 +905,9 @@ Example: `./manage.py create_federal_portfolio "AMTRAK" --parse_requests --parse | | Parameter | Description | |:-:|:-------------------------- |:-------------------------------------------------------------------------------------------| | 1 | **federal_agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | -| 2 | **both** | Optional. If True, runs parse_requests and parse_domains | -| 3 | **parse_requests** | Optional. If True, then the created portfolio is added to all related DomainRequests. | -| 4 | **parse_domains** | Optional. If True, then the created portfolio is added to all related Domains. | +| 2 | **both** | If True, runs parse_requests and parse_domains. | +| 3 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | +| 4 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | -Note: While you can specify both at the same time, you must specify either --parse_requests or --parse_domains. You cannot run the script without defining one or the other. +Note: Regarding parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them, +you must specify at least one to run this script. From 060dbfc66c01c326a6047a612b45512327480be6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 12 Sep 2024 08:39:01 -0600 Subject: [PATCH 60/65] More detailed logging on senior official --- .../commands/create_federal_portfolio.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index b98bb0c840..503b2c45f2 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -88,6 +88,16 @@ def create_or_modify_portfolio(self, federal_agency): if created: message = f"Created portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + + if portfolio_args.get("senior_official"): + message = f"Added senior official '{portfolio_args['senior_official']}'." + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + else: + message = ( + "No senior official added. " + "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`." + ) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) else: proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, @@ -114,6 +124,10 @@ def create_or_modify_portfolio(self, federal_agency): message = f"Modified portfolio '{portfolio}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + if portfolio_args.get("senior_official"): + message = f"Added/modified senior official '{portfolio_args['senior_official']}'." + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + return portfolio def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): From 87d35ebd7e15bef037d0db6285208a23ad64df08 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 12 Sep 2024 08:47:37 -0600 Subject: [PATCH 61/65] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 503b2c45f2..293050d036 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -94,10 +94,10 @@ def create_or_modify_portfolio(self, federal_agency): TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) else: message = ( - "No senior official added. " + f"No senior official added to portfolio '{portfolio}'. " "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`." ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) else: proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, @@ -126,7 +126,7 @@ def create_or_modify_portfolio(self, federal_agency): if portfolio_args.get("senior_official"): message = f"Added/modified senior official '{portfolio_args['senior_official']}'." - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) return portfolio From d787fcb39da6109c332f17f23ec178af42cc3cb7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 12 Sep 2024 08:50:21 -0600 Subject: [PATCH 62/65] Doc update --- docs/operations/data_migration.md | 9 +++++---- .../management/commands/create_federal_portfolio.py | 6 +++--- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 9bde4b6950..44696aba66 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -870,10 +870,11 @@ This script takes the name of a `FederalAgency` (like 'AMTRAK') and does the fol Errors: 1. ValueError: Federal agency not found in database. -2. Logged Error: No suborganizations found for agency. -3. Logged Warning: No new suborganizations to add. -4. Logged Warning: No valid DomainRequest records to update. -5. Logged Warning: No valid DomainInformation records to update. +2. Logged warning: No senior official found for portfolio +3. Logged Error: No suborganizations found for portfolio. +4. Logged Warning: No new suborganizations to add. +5. Logged Warning: No valid DomainRequest records to update. +6. Logged Warning: No valid DomainInformation records to update. ### Running on sandboxes diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 293050d036..d05a2911bb 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -90,12 +90,12 @@ def create_or_modify_portfolio(self, federal_agency): TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) if portfolio_args.get("senior_official"): - message = f"Added senior official '{portfolio_args['senior_official']}'." + message = f"Added senior official '{portfolio_args['senior_official']}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) else: message = ( f"No senior official added to portfolio '{portfolio}'. " - "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`." + "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" ) TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) else: @@ -125,7 +125,7 @@ def create_or_modify_portfolio(self, federal_agency): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) if portfolio_args.get("senior_official"): - message = f"Added/modified senior official '{portfolio_args['senior_official']}'." + message = f"Added/modified senior official '{portfolio_args['senior_official']}'" TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) return portfolio From 066e7965e4ea72b81617f2bc76b430794cf666e5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 12 Sep 2024 09:04:55 -0600 Subject: [PATCH 63/65] Update docs/operations/data_migration.md --- docs/operations/data_migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 44696aba66..5e1aa688ac 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -870,7 +870,7 @@ This script takes the name of a `FederalAgency` (like 'AMTRAK') and does the fol Errors: 1. ValueError: Federal agency not found in database. -2. Logged warning: No senior official found for portfolio +2. Logged Warning: No senior official found for portfolio 3. Logged Error: No suborganizations found for portfolio. 4. Logged Warning: No new suborganizations to add. 5. Logged Warning: No valid DomainRequest records to update. From 519ca82ee304cd4486f38a1982c645cb45001bb7 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 10:21:30 -0500 Subject: [PATCH 64/65] Remove type checking that was causing linter issues --- src/epplibwrapper/cert.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/epplibwrapper/cert.py b/src/epplibwrapper/cert.py index 15ff16c06f..9abcbee356 100644 --- a/src/epplibwrapper/cert.py +++ b/src/epplibwrapper/cert.py @@ -1,7 +1,7 @@ import os import tempfile -from django.conf import settings +from django.conf import settings # type: ignore class Cert: @@ -12,7 +12,7 @@ class Cert: variable but Python's ssl library requires a file. """ - def __init__(self, data=settings.SECRET_REGISTRY_CERT) -> None: + def __init__(self, data = settings.SECRET_REGISTRY_CERT) -> None: # type: ignore self.filename = self._write(data) def __del__(self): @@ -31,4 +31,4 @@ class Key(Cert): """Location of private key as written to disk.""" def __init__(self) -> None: - super().__init__(data=settings.SECRET_REGISTRY_KEY) + super().__init__(data=settings.SECRET_REGISTRY_KEY) # type: ignore From 529178cd84b272182587620a5b6d8047c4beeeb1 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Thu, 12 Sep 2024 12:55:12 -0500 Subject: [PATCH 65/65] linter error fixes --- src/epplibwrapper/cert.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/cert.py b/src/epplibwrapper/cert.py index 9abcbee356..589736a042 100644 --- a/src/epplibwrapper/cert.py +++ b/src/epplibwrapper/cert.py @@ -12,7 +12,7 @@ class Cert: variable but Python's ssl library requires a file. """ - def __init__(self, data = settings.SECRET_REGISTRY_CERT) -> None: # type: ignore + def __init__(self, data=settings.SECRET_REGISTRY_CERT) -> None: # type: ignore self.filename = self._write(data) def __del__(self):