mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-07-27 04:58:42 +02:00
Normalize name and clear fed agency in some cases
This commit is contained in:
parent
e55535b19d
commit
3a356c6055
5 changed files with 127 additions and 8 deletions
|
@ -5,6 +5,7 @@ import logging
|
||||||
from django.core.management import BaseCommand, CommandError
|
from django.core.management import BaseCommand, CommandError
|
||||||
from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper
|
from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper
|
||||||
from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User
|
from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User
|
||||||
|
from registrar.models.utility.generic_helper import normalize_string
|
||||||
|
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
@ -51,6 +52,11 @@ class Command(BaseCommand):
|
||||||
action=argparse.BooleanOptionalAction,
|
action=argparse.BooleanOptionalAction,
|
||||||
help="Adds portfolio to both requests and domains",
|
help="Adds portfolio to both requests and domains",
|
||||||
)
|
)
|
||||||
|
parser.add_argument(
|
||||||
|
"--include_started_requests",
|
||||||
|
action=argparse.BooleanOptionalAction,
|
||||||
|
help="If parse_requests is enabled, we parse started",
|
||||||
|
)
|
||||||
|
|
||||||
def handle(self, **options):
|
def handle(self, **options):
|
||||||
agency_name = options.get("agency_name")
|
agency_name = options.get("agency_name")
|
||||||
|
@ -58,6 +64,7 @@ class Command(BaseCommand):
|
||||||
parse_requests = options.get("parse_requests")
|
parse_requests = options.get("parse_requests")
|
||||||
parse_domains = options.get("parse_domains")
|
parse_domains = options.get("parse_domains")
|
||||||
both = options.get("both")
|
both = options.get("both")
|
||||||
|
include_started_requests = options.get("include_started_requests")
|
||||||
|
|
||||||
if not both:
|
if not both:
|
||||||
if not parse_requests and not parse_domains:
|
if not parse_requests and not parse_domains:
|
||||||
|
@ -66,6 +73,9 @@ class Command(BaseCommand):
|
||||||
if parse_requests or parse_domains:
|
if parse_requests or parse_domains:
|
||||||
raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.")
|
raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.")
|
||||||
|
|
||||||
|
if include_started_requests and not parse_requests:
|
||||||
|
raise CommandError("You must pass --parse_requests when using --include_started_requests")
|
||||||
|
|
||||||
federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch}
|
federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch}
|
||||||
agencies = FederalAgency.objects.filter(**federal_agency_filter)
|
agencies = FederalAgency.objects.filter(**federal_agency_filter)
|
||||||
if not agencies or agencies.count() < 1:
|
if not agencies or agencies.count() < 1:
|
||||||
|
@ -83,7 +93,9 @@ class Command(BaseCommand):
|
||||||
TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
|
TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
|
||||||
try:
|
try:
|
||||||
# C901 'Command.handle' is too complex (12)
|
# C901 'Command.handle' is too complex (12)
|
||||||
self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both)
|
self.handle_populate_portfolio(
|
||||||
|
federal_agency, parse_domains, parse_requests, both, include_started_requests
|
||||||
|
)
|
||||||
except Exception as exec:
|
except Exception as exec:
|
||||||
self.failed_portfolios.add(federal_agency)
|
self.failed_portfolios.add(federal_agency)
|
||||||
logger.error(exec)
|
logger.error(exec)
|
||||||
|
@ -99,7 +111,7 @@ class Command(BaseCommand):
|
||||||
display_as_str=True,
|
display_as_str=True,
|
||||||
)
|
)
|
||||||
|
|
||||||
def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both):
|
def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests):
|
||||||
"""Attempts to create a portfolio. If successful, this function will
|
"""Attempts to create a portfolio. If successful, this function will
|
||||||
also create new suborganizations"""
|
also create new suborganizations"""
|
||||||
portfolio, created = self.create_portfolio(federal_agency)
|
portfolio, created = self.create_portfolio(federal_agency)
|
||||||
|
@ -109,7 +121,7 @@ class Command(BaseCommand):
|
||||||
self.handle_portfolio_domains(portfolio, federal_agency)
|
self.handle_portfolio_domains(portfolio, federal_agency)
|
||||||
|
|
||||||
if parse_requests or both:
|
if parse_requests or both:
|
||||||
self.handle_portfolio_requests(portfolio, federal_agency)
|
self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests)
|
||||||
|
|
||||||
def create_portfolio(self, federal_agency):
|
def create_portfolio(self, federal_agency):
|
||||||
"""Creates a portfolio if it doesn't presently exist.
|
"""Creates a portfolio if it doesn't presently exist.
|
||||||
|
@ -182,7 +194,7 @@ class Command(BaseCommand):
|
||||||
for name in org_names - set(existing_suborgs.values_list("name", flat=True)):
|
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 ""
|
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():
|
if name is not None and normalize_string(name) == normalize_string(portfolio_name):
|
||||||
# You can use this to populate location information, when this occurs.
|
# You can use this to populate location information, when this occurs.
|
||||||
# However, this isn't needed for now so we can skip it.
|
# However, this isn't needed for now so we can skip it.
|
||||||
message = (
|
message = (
|
||||||
|
@ -191,7 +203,7 @@ class Command(BaseCommand):
|
||||||
)
|
)
|
||||||
TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message)
|
TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message)
|
||||||
else:
|
else:
|
||||||
new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore
|
new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore
|
||||||
|
|
||||||
if new_suborgs:
|
if new_suborgs:
|
||||||
Suborganization.objects.bulk_create(new_suborgs)
|
Suborganization.objects.bulk_create(new_suborgs)
|
||||||
|
@ -201,16 +213,18 @@ class Command(BaseCommand):
|
||||||
else:
|
else:
|
||||||
TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added")
|
TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added")
|
||||||
|
|
||||||
def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency):
|
def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, include_started_requests):
|
||||||
"""
|
"""
|
||||||
Associate portfolio with domain requests for a federal agency.
|
Associate portfolio with domain requests for a federal agency.
|
||||||
Updates all relevant domain request records.
|
Updates all relevant domain request records.
|
||||||
"""
|
"""
|
||||||
invalid_states = [
|
invalid_states = [
|
||||||
DomainRequest.DomainRequestStatus.STARTED,
|
|
||||||
DomainRequest.DomainRequestStatus.INELIGIBLE,
|
DomainRequest.DomainRequestStatus.INELIGIBLE,
|
||||||
DomainRequest.DomainRequestStatus.REJECTED,
|
DomainRequest.DomainRequestStatus.REJECTED,
|
||||||
]
|
]
|
||||||
|
if not include_started_requests:
|
||||||
|
invalid_states.append(DomainRequest.DomainRequestStatus.STARTED)
|
||||||
|
|
||||||
domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(
|
domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(
|
||||||
status__in=invalid_states
|
status__in=invalid_states
|
||||||
)
|
)
|
||||||
|
@ -229,7 +243,14 @@ class Command(BaseCommand):
|
||||||
# Get all suborg information and store it in a dict to avoid doing a db call
|
# 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")
|
suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name")
|
||||||
for domain_request in domain_requests:
|
for domain_request in domain_requests:
|
||||||
|
# Set the portfolio
|
||||||
domain_request.portfolio = portfolio
|
domain_request.portfolio = portfolio
|
||||||
|
|
||||||
|
# Conditionally clear federal agency if the org name is the same as the portfolio name.
|
||||||
|
if include_started_requests:
|
||||||
|
domain_request.sync_portfolio_and_federal_agency_for_started_requests()
|
||||||
|
|
||||||
|
# Set suborg info
|
||||||
if domain_request.organization_name in suborgs:
|
if domain_request.organization_name in suborgs:
|
||||||
domain_request.sub_organization = suborgs.get(domain_request.organization_name)
|
domain_request.sub_organization = suborgs.get(domain_request.organization_name)
|
||||||
else:
|
else:
|
||||||
|
@ -254,6 +275,7 @@ class Command(BaseCommand):
|
||||||
"requested_suborganization",
|
"requested_suborganization",
|
||||||
"suborganization_city",
|
"suborganization_city",
|
||||||
"suborganization_state_territory",
|
"suborganization_state_territory",
|
||||||
|
"federal_agency",
|
||||||
],
|
],
|
||||||
)
|
)
|
||||||
message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests."
|
message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests."
|
||||||
|
|
|
@ -8,7 +8,7 @@ from django_fsm import FSMField, transition # type: ignore
|
||||||
from django.utils import timezone
|
from django.utils import timezone
|
||||||
from registrar.models.domain import Domain
|
from registrar.models.domain import Domain
|
||||||
from registrar.models.federal_agency import FederalAgency
|
from registrar.models.federal_agency import FederalAgency
|
||||||
from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper
|
from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string
|
||||||
from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes
|
from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes
|
||||||
from registrar.utility.constants import BranchChoices
|
from registrar.utility.constants import BranchChoices
|
||||||
from auditlog.models import LogEntry
|
from auditlog.models import LogEntry
|
||||||
|
@ -729,6 +729,7 @@ class DomainRequest(TimeStampedModel):
|
||||||
"""Save override for custom properties"""
|
"""Save override for custom properties"""
|
||||||
self.sync_organization_type()
|
self.sync_organization_type()
|
||||||
self.sync_yes_no_form_fields()
|
self.sync_yes_no_form_fields()
|
||||||
|
self.sync_portfolio_and_federal_agency_for_started_requests()
|
||||||
|
|
||||||
if self._cached_status != self.status:
|
if self._cached_status != self.status:
|
||||||
self.last_status_update = timezone.now().date()
|
self.last_status_update = timezone.now().date()
|
||||||
|
@ -744,6 +745,22 @@ class DomainRequest(TimeStampedModel):
|
||||||
# Update the cached values after saving
|
# Update the cached values after saving
|
||||||
self._cache_status_and_status_reasons()
|
self._cache_status_and_status_reasons()
|
||||||
|
|
||||||
|
def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool:
|
||||||
|
"""
|
||||||
|
Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio.
|
||||||
|
Only runs on STARTED requests.
|
||||||
|
|
||||||
|
Returns a bool indicating if the federal agency was changed or not.
|
||||||
|
"""
|
||||||
|
agency_name = getattr(self.federal_agency, "agency", None)
|
||||||
|
portfolio_name = getattr(self.portfolio, "organization_name", None)
|
||||||
|
if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED:
|
||||||
|
if normalize_string(agency_name) == normalize_string(portfolio_name):
|
||||||
|
self.federal_agency = None
|
||||||
|
return True
|
||||||
|
|
||||||
|
return False
|
||||||
|
|
||||||
def create_requested_suborganization(self):
|
def create_requested_suborganization(self):
|
||||||
"""Creates the requested suborganization.
|
"""Creates the requested suborganization.
|
||||||
Adds the name, portfolio, city, and state_territory fields.
|
Adds the name, portfolio, city, and state_territory fields.
|
||||||
|
|
|
@ -343,3 +343,8 @@ def value_of_attribute(obj, attribute_name: str):
|
||||||
if callable(value):
|
if callable(value):
|
||||||
value = value()
|
value = value()
|
||||||
return value
|
return value
|
||||||
|
|
||||||
|
def normalize_string(string_to_normalize: str, lowercase=True) -> str:
|
||||||
|
"""Normalizes a given string. Returns a string without extra spaces, in all lowercase."""
|
||||||
|
new_string = " ".join(string_to_normalize.split())
|
||||||
|
return new_string.lower() if lowercase else new_string
|
||||||
|
|
|
@ -1424,6 +1424,7 @@ class TestCreateFederalPortfolio(TestCase):
|
||||||
|
|
||||||
# Create an agency wih no federal type (can only be created via specifiying it manually)
|
# Create an agency wih no federal type (can only be created via specifiying it manually)
|
||||||
self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency")
|
self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency")
|
||||||
|
self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane")
|
||||||
|
|
||||||
# And create some with federal_type ones with creative names
|
# And create some with federal_type ones with creative names
|
||||||
self.executive_agency_1 = FederalAgency.objects.create(
|
self.executive_agency_1 = FederalAgency.objects.create(
|
||||||
|
@ -1516,6 +1517,48 @@ class TestCreateFederalPortfolio(TestCase):
|
||||||
):
|
):
|
||||||
call_command("create_federal_portfolio", **kwargs)
|
call_command("create_federal_portfolio", **kwargs)
|
||||||
|
|
||||||
|
@less_console_noise_decorator
|
||||||
|
def test_handle_portfolio_requests_sync_federal_agency(self):
|
||||||
|
"""Test that federal agency is cleared when org name matches portfolio name"""
|
||||||
|
|
||||||
|
# Create a domain request with matching org name
|
||||||
|
matching_request = completed_domain_request(
|
||||||
|
name="matching.gov",
|
||||||
|
status=DomainRequest.DomainRequestStatus.STARTED,
|
||||||
|
generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
|
||||||
|
federal_agency=self.federal_agency_2,
|
||||||
|
user=self.user,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Create a request not in started (no change should occur)
|
||||||
|
matching_request_in_wrong_status = completed_domain_request(
|
||||||
|
name="kinda-matching.gov",
|
||||||
|
status=DomainRequest.DomainRequestStatus.IN_REVIEW,
|
||||||
|
generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
|
||||||
|
federal_agency=self.federal_agency,
|
||||||
|
user=self.user,
|
||||||
|
)
|
||||||
|
|
||||||
|
self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True)
|
||||||
|
self.run_create_federal_portfolio(
|
||||||
|
agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True
|
||||||
|
)
|
||||||
|
|
||||||
|
# Refresh from db
|
||||||
|
matching_request.refresh_from_db()
|
||||||
|
matching_request_in_wrong_status.refresh_from_db()
|
||||||
|
|
||||||
|
# Request with matching name should have federal_agency cleared
|
||||||
|
self.assertIsNone(matching_request.federal_agency)
|
||||||
|
self.assertIsNotNone(matching_request.portfolio)
|
||||||
|
self.assertEqual(matching_request.portfolio.organization_name, "Sugarcane")
|
||||||
|
|
||||||
|
# Request with matching name but wrong state should keep its federal agency
|
||||||
|
self.assertEqual(matching_request_in_wrong_status.federal_agency, self.federal_agency)
|
||||||
|
self.assertIsNotNone(matching_request_in_wrong_status.portfolio)
|
||||||
|
self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency")
|
||||||
|
|
||||||
|
@less_console_noise_decorator
|
||||||
def test_create_single_portfolio(self):
|
def test_create_single_portfolio(self):
|
||||||
"""Test portfolio creation with suborg and senior official."""
|
"""Test portfolio creation with suborg and senior official."""
|
||||||
self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
|
self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
|
||||||
|
|
|
@ -1074,6 +1074,38 @@ class TestDomainRequest(TestCase):
|
||||||
self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type)
|
self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type)
|
||||||
self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency)
|
self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency)
|
||||||
|
|
||||||
|
def test_sync_portfolio_and_federal_agency_for_started_requests(self):
|
||||||
|
"""Tests the sync_portfolio_and_federal_agency_for_started_requests function"""
|
||||||
|
# Create a federal agency with a "bad" name
|
||||||
|
user = create_user()
|
||||||
|
federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ")
|
||||||
|
request = completed_domain_request(
|
||||||
|
name="matching.gov",
|
||||||
|
status=DomainRequest.DomainRequestStatus.STARTED,
|
||||||
|
generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
|
||||||
|
federal_agency=federal_agency_2,
|
||||||
|
user=user,
|
||||||
|
)
|
||||||
|
self.assertIsNone(request.portfolio)
|
||||||
|
|
||||||
|
# Nothing should happen on normal save
|
||||||
|
request.notes = "test change"
|
||||||
|
request.save()
|
||||||
|
self.assertEqual(request.federal_agency, federal_agency_2)
|
||||||
|
|
||||||
|
# But when a portfolio exists, the federal agency should be cleared if its a duplicate
|
||||||
|
portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user)
|
||||||
|
request.portfolio = portfolio
|
||||||
|
request.save()
|
||||||
|
self.assertIsNone(request.federal_agency)
|
||||||
|
|
||||||
|
# However -- this change should only occur if the names match (when normalized)
|
||||||
|
portfolio = Portfolio.objects.create(organization_name="some other name", creator=user)
|
||||||
|
request.portfolio = portfolio
|
||||||
|
request.federal_agency = federal_agency_2
|
||||||
|
request.save()
|
||||||
|
self.assertEqual(request.federal_agency, federal_agency_2)
|
||||||
|
|
||||||
|
|
||||||
class TestDomainRequestSuborganization(TestCase):
|
class TestDomainRequestSuborganization(TestCase):
|
||||||
"""Tests for the suborganization fields on domain requests"""
|
"""Tests for the suborganization fields on domain requests"""
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue