From 77e5c918e7cacf5cba4e216dbaeae9ccf77c261e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 4 Jun 2024 11:51:39 -0400 Subject: [PATCH 01/25] added skipEppSave command line option, may change option in future commit --- src/registrar/admin.py | 48 +++++++++++++++---- .../management/commands/import_tables.py | 9 +++- 2 files changed, 46 insertions(+), 11 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ce00bbc88..6573079f9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2252,18 +2252,46 @@ class PublicContactResource(resources.ModelResource): class Meta: model = models.PublicContact - def import_row(self, row, instance_loader, using_transactions=True, dry_run=False, raise_errors=None, **kwargs): - """Override kwargs skip_epp_save and set to True""" - kwargs["skip_epp_save"] = True - return super().import_row( - row, - instance_loader, - using_transactions=using_transactions, - dry_run=dry_run, - raise_errors=raise_errors, + def __init__(self): + """Sets global variables for code tidyness""" + super().__init__() + self.skip_epp_save=False + + def import_data( + self, + dataset, + dry_run=False, + raise_errors=False, + use_transactions=None, + collect_failed_rows=False, + rollback_on_validation_errors=False, + **kwargs + ): + """Override import_data to set self.skip_epp_save if in kwargs""" + self.skip_epp_save = kwargs.get('skip_epp_save', False) + return super().import_data( + dataset, + dry_run, + raise_errors, + use_transactions, + collect_failed_rows, + rollback_on_validation_errors, **kwargs, ) + # def import_row(self, row, instance_loader, using_transactions=True, dry_run=False, raise_errors=None, **kwargs): + # """Override kwargs skip_epp_save and set to True""" + # kwargs["skip_epp_save"] = True + # super().import_data() + # return super().import_row( + # row, + # instance_loader, + # using_transactions=using_transactions, + # dry_run=dry_run, + # raise_errors=raise_errors, + # **kwargs, + # ) + def save_instance(self, instance, is_create, using_transactions=True, dry_run=False): """Override save_instance setting skip_epp_save to True""" self.before_save_instance(instance, using_transactions, dry_run) @@ -2276,7 +2304,7 @@ class PublicContactResource(resources.ModelResource): # we don't have transactions and we want to do a dry_run pass else: - instance.save(skip_epp_save=True) + instance.save(skip_epp_save=self.skip_epp_save) self.after_save_instance(instance, using_transactions, dry_run) diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index 3594d3215..cbe3dc6ee 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -1,3 +1,4 @@ +import argparse import logging import os import pyzipper @@ -14,6 +15,10 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = "Imports tables from a zip file, exported_tables.zip, containing CSV files in the tmp directory." + def add_arguments(self, parser): + """Add command line arguments.""" + parser.add_argument('--skipEppSave', default=True, action=argparse.BooleanOptionalAction) + def handle(self, **options): """Extracts CSV files from a zip archive and imports them into the respective tables""" @@ -21,6 +26,8 @@ class Command(BaseCommand): logger.error("import_tables cannot be run in production") return + self.skip_epp_save = options.get("skipEppSave") + table_names = [ "User", "Contact", @@ -73,7 +80,7 @@ class Command(BaseCommand): resource_instance = resourceclass() with open(csv_filename, "r") as csvfile: dataset = tablib.Dataset().load(csvfile.read(), format="csv") - result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=True) + result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) if result.has_errors(): logger.error(f"Errors occurred while importing {csv_filename}: {result.row_errors()}") From 5adf0630844fb798c74f6c899834081d471efd80 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 6 Jun 2024 16:56:48 -0600 Subject: [PATCH 02/25] Initial scaffold of models (still needs logic for auto-populating some fields) --- src/registrar/models/organization.py | 145 +++++++++++++++++++++++++++ src/registrar/models/portfolio.py | 109 ++++++++++++++++++++ 2 files changed, 254 insertions(+) create mode 100644 src/registrar/models/organization.py create mode 100644 src/registrar/models/portfolio.py diff --git a/src/registrar/models/organization.py b/src/registrar/models/organization.py new file mode 100644 index 000000000..3ae54719e --- /dev/null +++ b/src/registrar/models/organization.py @@ -0,0 +1,145 @@ +from django.db import models +from django_fsm import FSMField # type: ignore + +from registrar.models.domain_request import DomainRequest +from registrar.models.federal_agency import FederalAgency +from registrar.models.domain import State + +from .utility.time_stamped_model import TimeStampedModel + + +class Organization(TimeStampedModel): + """ + TODO: + """ + + class Meta: + """Contains meta information about this class""" + + indexes = [ + models.Index(fields=["user"]), + models.Index(fields=["email"]), + ] + + # federal agency - FK to fed agency table (Not nullable, should default to the Non-federal agency value in the fed agency table) + federal_agency = models.ForeignKey( + "registrar.FederalAgency", + on_delete=models.PROTECT, + help_text="Associated federal agency", + unique=False, + default=FederalAgency.objects.filter(agency="Non-Federal Agency").first() + ) + + # creator- user foreign key- stores who created this model + # should get the user who is adding it via django admin if there + # is a user (aka not done via commandline/ manual means) + # TODO: auto-populate + creator = models.ForeignKey( + "registrar.User", + on_delete=models.PROTECT, + help_text="Associated user", + unique=False + ) + + # organization type- should match organization types allowed on domain info + organization_type = models.CharField( + max_length=255, + choices=DomainRequest.OrgChoicesElectionOffice.choices, + null=True, + blank=True, + help_text='"Election" appears after the org type if it\'s an election office.', + ) + + # organization name + # TODO: org name will be the same as federal agency, if it is federal, + # otherwise it will be the actual org name. If nothing is entered for + # org name and it is a federal organization, have this field fill with + # the federal agency text name. + organization_name = models.CharField( + null=True, + blank=True, + ) + + # address_line1 + address_line1 = models.CharField( + null=True, + blank=True, + verbose_name="address line 1", + ) + # address_line2 + address_line2 = models.CharField( + null=True, + blank=True, + verbose_name="address line 2", + ) + # city + city = models.CharField( + null=True, + blank=True, + ) + # state (copied from domain.py -- imports enums from domain.py) + state = FSMField( + max_length=21, + choices=State.choices, + default=State.UNKNOWN, + # cannot change state directly, particularly in Django admin + protected=True, + # This must be defined for custom state help messages, + # as otherwise the view will purge the help field as it does not exist. + help_text=" ", + verbose_name="domain state", + ) + # zipcode + zipcode = models.CharField( + max_length=10, + null=True, + blank=True, + verbose_name="zip code", + ) + # urbanization + urbanization = models.CharField( + null=True, + blank=True, + help_text="Required for Puerto Rico only", + verbose_name="urbanization", + ) + + # security_contact_email + security_contact_email = models.EmailField( + null=True, + blank=True, + verbose_name="security contact e-mail", + max_length=320, + ) + + # def save(self, *args, **kwargs): + # # Call the parent class's save method to perform the actual save + # super().save(*args, **kwargs) + + # if self.user: + # updated = False + + # # Update first name and last name if necessary + # if not self.user.first_name or not self.user.last_name: + # self.user.first_name = self.first_name + # self.user.last_name = self.last_name + # updated = True + + # # Update phone if necessary + # if not self.user.phone: + # self.user.phone = self.phone + # updated = True + + # # Save user if any updates were made + # if updated: + # self.user.save() + + # def __str__(self): + # if self.first_name or self.last_name: + # return self.get_formatted_name() + # elif self.email: + # return self.email + # elif self.pk: + # return str(self.pk) + # else: + # return "" diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py new file mode 100644 index 000000000..801f267f4 --- /dev/null +++ b/src/registrar/models/portfolio.py @@ -0,0 +1,109 @@ +from django.db import models + +from .utility.time_stamped_model import TimeStampedModel + + +class Portfolio(TimeStampedModel): + """ + TODO: + """ + + class Meta: + """Contains meta information about this class""" + + indexes = [ + models.Index(fields=["user"]), + models.Index(fields=["email"]), + ] + + # NOTE: This will be specified in a future ticket + # class PortfolioTypes(models.TextChoices): + # TYPE1 = "foo", "foo" + # TYPE2 = "foo", "foo" + # TYPE3 = "foo", "foo" + # TYPE4 = "foo", "foo" + + + # creator- user foreign key- stores who created this model should get the user who is adding + # it via django admin if there is a user (aka not done via commandline/ manual means)""" + # TODO: auto-populate + creator = models.ForeignKey( + "registrar.User", + on_delete=models.PROTECT, + help_text="Associated user", + unique=False + ) + + # NOTE: This will be specified in a future ticket + # portfolio_type = models.TextField( + # choices=PortfolioTypes.choices, + # null=True, + # blank=True, + # ) + + # notes- text field (copy what is done on requests/domains) + notes = models.TextField( + null=True, + blank=True, + ) + + # domains- many to many to Domain field (nullable) + domains = models.ManyToManyField( + "registrar.Domain", + null=True, + blank=True, + related_name="portfolio domains", + verbose_name="portfolio domains", + # on_delete=models.PROTECT, # TODO: protect this? + ) + + # domain_requests- Many to many to Domain Request field (nullable) + domain_requests = models.ManyToManyField( + "registrar.DomainRequest", + null=True, + blank=True, + related_name="portfolio domain requests", + verbose_name="portfolio domain requests", + # on_delete=models.PROTECT, # TODO: protect this? + ) + + # organization + organization = models.OneToOneField( + "registrar.Organization", + null=True, + blank=True, + # on_delete=models.PROTECT, # TODO: protect this? + ) + + + # def save(self, *args, **kwargs): + # # Call the parent class's save method to perform the actual save + # super().save(*args, **kwargs) + + # if self.user: + # updated = False + + # # Update first name and last name if necessary + # if not self.user.first_name or not self.user.last_name: + # self.user.first_name = self.first_name + # self.user.last_name = self.last_name + # updated = True + + # # Update phone if necessary + # if not self.user.phone: + # self.user.phone = self.phone + # updated = True + + # # Save user if any updates were made + # if updated: + # self.user.save() + + # def __str__(self): + # if self.first_name or self.last_name: + # return self.get_formatted_name() + # elif self.email: + # return self.email + # elif self.pk: + # return str(self.pk) + # else: + # return "" From b3a272025d52791add5655d40c82c5011df9f771 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 11 Jun 2024 07:09:11 -0400 Subject: [PATCH 03/25] update import_tables documentation --- docs/operations/import_export.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index 7ddfd5d3b..4810774e4 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -35,6 +35,7 @@ For reference, the zip file will contain the following tables in csv form: * Websites * Host * HostIP +* PublicContact After exporting the file from the target environment, scp the exported_tables.zip file from the target environment to local. Run the below commands from local. @@ -81,11 +82,18 @@ For reference, this deletes all rows from the following tables: * DraftDomain * HostIP * Host +* PublicContact #### Importing into Target Environment Once target environment is prepared, files can be imported. +If importing tables from stable environment into an OT&E sandbox, there will be a difference +between the stable's registry and the sandbox's registry. Therefore, you need to run import_tables +with --skipEppSave option set to False. If you set to False, it will attempt to save PublicContact +records to the registry on load. If this is unset, or set to True, it will load the database and not +attempt to update the registry on load. + To scp the exported_tables.zip file from local to the sandbox, run the following: Get passcode by running: @@ -107,7 +115,7 @@ cf ssh {target-app} example cleaning getgov-backup: cf ssh getgov-backup /tmp/lifecycle/backup -./manage.py import_tables +./manage.py import_tables --skipEppSave=False For reference, this imports tables in the following order: @@ -121,6 +129,7 @@ For reference, this imports tables in the following order: * DomainRequest * DomainInformation * UserDomainRole +* PublicContact Optional step: * Run fixtures to load fixture users back in \ No newline at end of file From 85f3234fcd60ee981ab11596fe458829b65cd7c9 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 12 Jun 2024 14:49:06 -0600 Subject: [PATCH 04/25] Model corrections --- src/registrar/models/domain_information.py | 11 ++ src/registrar/models/domain_request.py | 10 + src/registrar/models/organization.py | 145 --------------- src/registrar/models/portfolio.py | 207 ++++++++++++++++----- 4 files changed, 183 insertions(+), 190 deletions(-) delete mode 100644 src/registrar/models/organization.py diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index bc35d4e30..198bf29c5 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -57,6 +57,17 @@ class DomainInformation(TimeStampedModel): help_text="Person who submitted the domain request", ) + + # portfolio + portfolio = models.OneToOneField( + "registrar.Portfolio", + on_delete=models.PROTECT, + null=True, + blank=True, + related_name="DomainRequest_portfolio", + help_text="Portfolio associated with this domain", + ) + domain_request = models.OneToOneField( "registrar.DomainRequest", on_delete=models.PROTECT, diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 8dfa419c9..a594f29f5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -286,6 +286,16 @@ class DomainRequest(TimeStampedModel): null=True, ) + # portfolio + portfolio = models.OneToOneField( + "registrar.Portfolio", + on_delete=models.PROTECT, + null=True, + blank=True, + related_name="DomainInformation_portfolio", + help_text="Portfolio associated with this domain", + ) + # This is the domain request user who created this domain request. The contact # information that they gave is in the `submitter` field creator = models.ForeignKey( diff --git a/src/registrar/models/organization.py b/src/registrar/models/organization.py deleted file mode 100644 index 3ae54719e..000000000 --- a/src/registrar/models/organization.py +++ /dev/null @@ -1,145 +0,0 @@ -from django.db import models -from django_fsm import FSMField # type: ignore - -from registrar.models.domain_request import DomainRequest -from registrar.models.federal_agency import FederalAgency -from registrar.models.domain import State - -from .utility.time_stamped_model import TimeStampedModel - - -class Organization(TimeStampedModel): - """ - TODO: - """ - - class Meta: - """Contains meta information about this class""" - - indexes = [ - models.Index(fields=["user"]), - models.Index(fields=["email"]), - ] - - # federal agency - FK to fed agency table (Not nullable, should default to the Non-federal agency value in the fed agency table) - federal_agency = models.ForeignKey( - "registrar.FederalAgency", - on_delete=models.PROTECT, - help_text="Associated federal agency", - unique=False, - default=FederalAgency.objects.filter(agency="Non-Federal Agency").first() - ) - - # creator- user foreign key- stores who created this model - # should get the user who is adding it via django admin if there - # is a user (aka not done via commandline/ manual means) - # TODO: auto-populate - creator = models.ForeignKey( - "registrar.User", - on_delete=models.PROTECT, - help_text="Associated user", - unique=False - ) - - # organization type- should match organization types allowed on domain info - organization_type = models.CharField( - max_length=255, - choices=DomainRequest.OrgChoicesElectionOffice.choices, - null=True, - blank=True, - help_text='"Election" appears after the org type if it\'s an election office.', - ) - - # organization name - # TODO: org name will be the same as federal agency, if it is federal, - # otherwise it will be the actual org name. If nothing is entered for - # org name and it is a federal organization, have this field fill with - # the federal agency text name. - organization_name = models.CharField( - null=True, - blank=True, - ) - - # address_line1 - address_line1 = models.CharField( - null=True, - blank=True, - verbose_name="address line 1", - ) - # address_line2 - address_line2 = models.CharField( - null=True, - blank=True, - verbose_name="address line 2", - ) - # city - city = models.CharField( - null=True, - blank=True, - ) - # state (copied from domain.py -- imports enums from domain.py) - state = FSMField( - max_length=21, - choices=State.choices, - default=State.UNKNOWN, - # cannot change state directly, particularly in Django admin - protected=True, - # This must be defined for custom state help messages, - # as otherwise the view will purge the help field as it does not exist. - help_text=" ", - verbose_name="domain state", - ) - # zipcode - zipcode = models.CharField( - max_length=10, - null=True, - blank=True, - verbose_name="zip code", - ) - # urbanization - urbanization = models.CharField( - null=True, - blank=True, - help_text="Required for Puerto Rico only", - verbose_name="urbanization", - ) - - # security_contact_email - security_contact_email = models.EmailField( - null=True, - blank=True, - verbose_name="security contact e-mail", - max_length=320, - ) - - # def save(self, *args, **kwargs): - # # Call the parent class's save method to perform the actual save - # super().save(*args, **kwargs) - - # if self.user: - # updated = False - - # # Update first name and last name if necessary - # if not self.user.first_name or not self.user.last_name: - # self.user.first_name = self.first_name - # self.user.last_name = self.last_name - # updated = True - - # # Update phone if necessary - # if not self.user.phone: - # self.user.phone = self.phone - # updated = True - - # # Save user if any updates were made - # if updated: - # self.user.save() - - # def __str__(self): - # if self.first_name or self.last_name: - # return self.get_formatted_name() - # elif self.email: - # return self.email - # elif self.pk: - # return str(self.pk) - # else: - # return "" diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 801f267f4..6721a8a20 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -1,4 +1,9 @@ from django.db import models +from django_fsm import FSMField # type: ignore + +from registrar.models.domain_request import DomainRequest +from registrar.models.federal_agency import FederalAgency +from registrar.models.domain import State from .utility.time_stamped_model import TimeStampedModel @@ -15,6 +20,10 @@ class Portfolio(TimeStampedModel): models.Index(fields=["user"]), models.Index(fields=["email"]), ] + + + # use the short names in Django admin + OrganizationChoices = DomainRequest.OrganizationChoices # NOTE: This will be specified in a future ticket # class PortfolioTypes(models.TextChoices): @@ -23,6 +32,12 @@ class Portfolio(TimeStampedModel): # TYPE3 = "foo", "foo" # TYPE4 = "foo", "foo" + # NOTE: This will be specified in a future ticket + # portfolio_type = models.TextField( + # choices=PortfolioTypes.choices, + # null=True, + # blank=True, + # ) # creator- user foreign key- stores who created this model should get the user who is adding # it via django admin if there is a user (aka not done via commandline/ manual means)""" @@ -34,69 +49,171 @@ class Portfolio(TimeStampedModel): unique=False ) - # NOTE: This will be specified in a future ticket - # portfolio_type = models.TextField( - # choices=PortfolioTypes.choices, - # null=True, - # blank=True, - # ) - # notes- text field (copy what is done on requests/domains) notes = models.TextField( null=True, blank=True, ) - # domains- many to many to Domain field (nullable) - domains = models.ManyToManyField( - "registrar.Domain", - null=True, - blank=True, - related_name="portfolio domains", - verbose_name="portfolio domains", - # on_delete=models.PROTECT, # TODO: protect this? + # # domains- many to many to Domain field (nullable) + # domains = models.ManyToManyField( + # "registrar.Domain", + # null=True, + # blank=True, + # related_name="portfolio domains", + # verbose_name="portfolio domains", + # # on_delete=models.PROTECT, # TODO: protect this? + # ) + + # # domain_requests- Many to many to Domain Request field (nullable) + # domain_requests = models.ManyToManyField( + # "registrar.DomainRequest", + # null=True, + # blank=True, + # related_name="portfolio domain requests", + # verbose_name="portfolio domain requests", + # # on_delete=models.PROTECT, # TODO: protect this? + # ) + + # # organization + # organization = models.OneToOneField( + # "registrar.Organization", + # null=True, + # blank=True, + # ) + + + # federal agency - FK to fed agency table (Not nullable, should default to the Non-federal agency value in the fed agency table) + federal_agency = models.ForeignKey( + "registrar.FederalAgency", + on_delete=models.PROTECT, + help_text="Associated federal agency", + unique=False, + default=FederalAgency.objects.filter(agency="Non-Federal Agency").first() ) - # domain_requests- Many to many to Domain Request field (nullable) - domain_requests = models.ManyToManyField( - "registrar.DomainRequest", - null=True, - blank=True, - related_name="portfolio domain requests", - verbose_name="portfolio domain requests", - # on_delete=models.PROTECT, # TODO: protect this? + # creator- user foreign key- stores who created this model + # should get the user who is adding it via django admin if there + # is a user (aka not done via commandline/ manual means) + # TODO: auto-populate + creator = models.ForeignKey( + "registrar.User", + on_delete=models.PROTECT, + help_text="Associated user", + unique=False ) - # organization - organization = models.OneToOneField( - "registrar.Organization", + # organization type- should match organization types allowed on domain info + organization_type = models.CharField( + max_length=255, + choices=OrganizationChoices.choices, null=True, blank=True, - # on_delete=models.PROTECT, # TODO: protect this? + help_text="Type of organization", ) - - # def save(self, *args, **kwargs): - # # Call the parent class's save method to perform the actual save - # super().save(*args, **kwargs) + # organization name + # TODO: org name will be the same as federal agency, if it is federal, + # otherwise it will be the actual org name. If nothing is entered for + # org name and it is a federal organization, have this field fill with + # the federal agency text name. + organization_name = models.CharField( + null=True, + blank=True, + ) - # if self.user: - # updated = False + # address_line1 + address_line1 = models.CharField( + null=True, + blank=True, + verbose_name="address line 1", + ) + # address_line2 + address_line2 = models.CharField( + null=True, + blank=True, + verbose_name="address line 2", + ) + # city + city = models.CharField( + null=True, + blank=True, + ) + # state (copied from domain.py -- imports enums from domain.py) + state = FSMField( + max_length=21, + choices=State.choices, + default=State.UNKNOWN, + # cannot change state directly, particularly in Django admin + protected=True, + # This must be defined for custom state help messages, + # as otherwise the view will purge the help field as it does not exist. + help_text=" ", + verbose_name="domain state", + ) + # zipcode + zipcode = models.CharField( + max_length=10, + null=True, + blank=True, + verbose_name="zip code", + ) + # urbanization + urbanization = models.CharField( + null=True, + blank=True, + help_text="Required for Puerto Rico only", + verbose_name="urbanization", + ) - # # Update first name and last name if necessary - # if not self.user.first_name or not self.user.last_name: - # self.user.first_name = self.first_name - # self.user.last_name = self.last_name - # updated = True + # security_contact_email + security_contact_email = models.EmailField( + null=True, + blank=True, + verbose_name="security contact e-mail", + max_length=320, + ) - # # Update phone if necessary - # if not self.user.phone: - # self.user.phone = self.phone - # updated = True + def save(self, *args, **kwargs): + # Call the parent class's save method to perform the actual save + super().save(*args, **kwargs) - # # Save user if any updates were made - # if updated: - # self.user.save() + # TODO: + # ---- auto-populate creator ---- + # creator- user foreign key- stores who created this model + # should get the user who is adding it via django admin if there + # is a user (aka not done via commandline/ manual means) + + # ---- update organization name ---- + # org name will be the same as federal agency, if it is federal, + # otherwise it will be the actual org name. If nothing is entered for + # org name and it is a federal organization, have this field fill with + # the federal agency text name. + is_federal = self.organization_type == self.OrganizationChoices.FEDERAL + if is_federal: + self.organization_name = DomainRequest.OrganizationChoicesVerbose(self.organization_type) + #NOTE: Is this what is meant by "federal agency text name?" + + + # ----------------------------------- + # if self.user: + # updated = False + + # # Update first name and last name if necessary + # if not self.user.first_name or not self.user.last_name: + # self.user.first_name = self.first_name + # self.user.last_name = self.last_name + # updated = True + + # # Update phone if necessary + # if not self.user.phone: + # self.user.phone = self.phone + # updated = True + + # # Save user if any updates were made + # if updated: + # self.user.save() + # ----------------------------------- # def __str__(self): # if self.first_name or self.last_name: From c9c6890e7d6a7ef62299e658b23c65b52d988f0c Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 12 Jun 2024 15:47:13 -0600 Subject: [PATCH 05/25] model updates --- src/registrar/admin.py | 17 +++++++ src/registrar/models/domain_information.py | 16 +++---- src/registrar/models/domain_request.py | 16 +++---- src/registrar/models/portfolio.py | 53 +--------------------- 4 files changed, 35 insertions(+), 67 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 6aabfdd52..578146624 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2464,6 +2464,23 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): super().save_model(request, obj, form, change) +class PortfolioAdmin(ListHeaderAdmin): + # list_display = ("email", "requestor", "truncated_notes", "created_at") + # search_fields = ["email"] + # search_help_text = "Search by email." + # readonly_fields = [ + # "requestor", + # ] + + # change_form_template = "django/admin/email_clipboard_change_form.html" + + def save_model(self, request, obj, form, change): + # Set the creator field to the current admin user + obj.creator = request.user if request.user.is_authenticated else None + super().save_model(request, obj, form, change) + + + class FederalAgencyAdmin(ListHeaderAdmin): list_display = ["agency"] search_fields = ["agency"] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 198bf29c5..8235981f5 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -59,14 +59,14 @@ class DomainInformation(TimeStampedModel): # portfolio - portfolio = models.OneToOneField( - "registrar.Portfolio", - on_delete=models.PROTECT, - null=True, - blank=True, - related_name="DomainRequest_portfolio", - help_text="Portfolio associated with this domain", - ) + # portfolio = models.OneToOneField( + # "registrar.Portfolio", + # on_delete=models.PROTECT, + # null=True, + # blank=True, + # related_name="DomainRequest_portfolio", + # help_text="Portfolio associated with this domain", + # ) domain_request = models.OneToOneField( "registrar.DomainRequest", diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index a594f29f5..8c0734ba9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -287,14 +287,14 @@ class DomainRequest(TimeStampedModel): ) # portfolio - portfolio = models.OneToOneField( - "registrar.Portfolio", - on_delete=models.PROTECT, - null=True, - blank=True, - related_name="DomainInformation_portfolio", - help_text="Portfolio associated with this domain", - ) + # portfolio = models.OneToOneField( + # "registrar.Portfolio", + # on_delete=models.PROTECT, + # null=True, + # blank=True, + # related_name="DomainInformation_portfolio", + # help_text="Portfolio associated with this domain", + # ) # This is the domain request user who created this domain request. The contact # information that they gave is in the `submitter` field diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 6721a8a20..cbd39f3c7 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -10,34 +10,13 @@ from .utility.time_stamped_model import TimeStampedModel class Portfolio(TimeStampedModel): """ - TODO: + Portfolio is used for organizing domains/domain-requests into + manageable groups. """ - - class Meta: - """Contains meta information about this class""" - - indexes = [ - models.Index(fields=["user"]), - models.Index(fields=["email"]), - ] - # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices - - # NOTE: This will be specified in a future ticket - # class PortfolioTypes(models.TextChoices): - # TYPE1 = "foo", "foo" - # TYPE2 = "foo", "foo" - # TYPE3 = "foo", "foo" - # TYPE4 = "foo", "foo" - # NOTE: This will be specified in a future ticket - # portfolio_type = models.TextField( - # choices=PortfolioTypes.choices, - # null=True, - # blank=True, - # ) # creator- user foreign key- stores who created this model should get the user who is adding # it via django admin if there is a user (aka not done via commandline/ manual means)""" @@ -54,34 +33,6 @@ class Portfolio(TimeStampedModel): null=True, blank=True, ) - - # # domains- many to many to Domain field (nullable) - # domains = models.ManyToManyField( - # "registrar.Domain", - # null=True, - # blank=True, - # related_name="portfolio domains", - # verbose_name="portfolio domains", - # # on_delete=models.PROTECT, # TODO: protect this? - # ) - - # # domain_requests- Many to many to Domain Request field (nullable) - # domain_requests = models.ManyToManyField( - # "registrar.DomainRequest", - # null=True, - # blank=True, - # related_name="portfolio domain requests", - # verbose_name="portfolio domain requests", - # # on_delete=models.PROTECT, # TODO: protect this? - # ) - - # # organization - # organization = models.OneToOneField( - # "registrar.Organization", - # null=True, - # blank=True, - # ) - # federal agency - FK to fed agency table (Not nullable, should default to the Non-federal agency value in the fed agency table) federal_agency = models.ForeignKey( From de926b1abbea873095b47284a440e30fb84d9bb3 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 12 Jun 2024 17:05:22 -0600 Subject: [PATCH 06/25] migrations --- src/registrar/admin.py | 22 ++- ...io_domaininformation_portfolio_and_more.py | 174 ++++++++++++++++++ .../migrations/0102_create_groups_v13.py | 37 ++++ src/registrar/models/__init__.py | 3 + src/registrar/models/domain_information.py | 16 +- src/registrar/models/domain_request.py | 16 +- src/registrar/models/portfolio.py | 93 ++-------- 7 files changed, 261 insertions(+), 100 deletions(-) create mode 100644 src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py create mode 100644 src/registrar/migrations/0102_create_groups_v13.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 578146624..9a1110af4 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2465,18 +2465,29 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): class PortfolioAdmin(ListHeaderAdmin): - # list_display = ("email", "requestor", "truncated_notes", "created_at") - # search_fields = ["email"] - # search_help_text = "Search by email." + # NOTE: these are just placeholders. Not part of ACs (haven't been defined yet). Update in future tickets. + list_display = ("organization_name", "federal_agency", "creator") + search_fields = ["organization_name"] + search_help_text = "Search by organization name." # readonly_fields = [ # "requestor", # ] - # change_form_template = "django/admin/email_clipboard_change_form.html" - def save_model(self, request, obj, form, change): + # ---- update creator ---- # Set the creator field to the current admin user obj.creator = request.user if request.user.is_authenticated else None + + # ---- update organization name ---- + # org name will be the same as federal agency, if it is federal, + # otherwise it will be the actual org name. If nothing is entered for + # org name and it is a federal organization, have this field fill with + # the federal agency text name. + is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL + if is_federal: + obj.organization_name = obj.organization_type + #NOTE: What is meant by "federal agency text name?" + super().save_model(request, obj, form, change) @@ -2551,6 +2562,7 @@ admin.site.register(models.PublicContact, PublicContactAdmin) admin.site.register(models.DomainRequest, DomainRequestAdmin) admin.site.register(models.TransitionDomain, TransitionDomainAdmin) admin.site.register(models.VerifiedByStaff, VerifiedByStaffAdmin) +admin.site.register(models.Portfolio, PortfolioAdmin) # Register our custom waffle implementations admin.site.register(models.WaffleFlag, WaffleFlagAdmin) diff --git a/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py new file mode 100644 index 000000000..fd2138801 --- /dev/null +++ b/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py @@ -0,0 +1,174 @@ +# Generated by Django 4.2.10 on 2024-06-12 22:15 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import registrar.models.portfolio + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0100_domainrequest_action_needed_reason"), + ] + + operations = [ + migrations.CreateModel( + name="Portfolio", + fields=[ + ("id", models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name="ID")), + ("created_at", models.DateTimeField(auto_now_add=True)), + ("updated_at", models.DateTimeField(auto_now=True)), + ("notes", models.TextField(blank=True, null=True)), + ( + "organization_type", + models.CharField( + blank=True, + choices=[ + ("federal", "Federal"), + ("interstate", "Interstate"), + ("state_or_territory", "State or territory"), + ("tribal", "Tribal"), + ("county", "County"), + ("city", "City"), + ("special_district", "Special district"), + ("school_district", "School district"), + ], + help_text="Type of organization", + max_length=255, + null=True, + ), + ), + ("organization_name", models.CharField(blank=True, null=True)), + ("address_line1", models.CharField(blank=True, null=True, verbose_name="address line 1")), + ("address_line2", models.CharField(blank=True, null=True, verbose_name="address line 2")), + ("city", models.CharField(blank=True, null=True)), + ( + "state_territory", + models.CharField( + blank=True, + choices=[ + ("AL", "Alabama (AL)"), + ("AK", "Alaska (AK)"), + ("AS", "American Samoa (AS)"), + ("AZ", "Arizona (AZ)"), + ("AR", "Arkansas (AR)"), + ("CA", "California (CA)"), + ("CO", "Colorado (CO)"), + ("CT", "Connecticut (CT)"), + ("DE", "Delaware (DE)"), + ("DC", "District of Columbia (DC)"), + ("FL", "Florida (FL)"), + ("GA", "Georgia (GA)"), + ("GU", "Guam (GU)"), + ("HI", "Hawaii (HI)"), + ("ID", "Idaho (ID)"), + ("IL", "Illinois (IL)"), + ("IN", "Indiana (IN)"), + ("IA", "Iowa (IA)"), + ("KS", "Kansas (KS)"), + ("KY", "Kentucky (KY)"), + ("LA", "Louisiana (LA)"), + ("ME", "Maine (ME)"), + ("MD", "Maryland (MD)"), + ("MA", "Massachusetts (MA)"), + ("MI", "Michigan (MI)"), + ("MN", "Minnesota (MN)"), + ("MS", "Mississippi (MS)"), + ("MO", "Missouri (MO)"), + ("MT", "Montana (MT)"), + ("NE", "Nebraska (NE)"), + ("NV", "Nevada (NV)"), + ("NH", "New Hampshire (NH)"), + ("NJ", "New Jersey (NJ)"), + ("NM", "New Mexico (NM)"), + ("NY", "New York (NY)"), + ("NC", "North Carolina (NC)"), + ("ND", "North Dakota (ND)"), + ("MP", "Northern Mariana Islands (MP)"), + ("OH", "Ohio (OH)"), + ("OK", "Oklahoma (OK)"), + ("OR", "Oregon (OR)"), + ("PA", "Pennsylvania (PA)"), + ("PR", "Puerto Rico (PR)"), + ("RI", "Rhode Island (RI)"), + ("SC", "South Carolina (SC)"), + ("SD", "South Dakota (SD)"), + ("TN", "Tennessee (TN)"), + ("TX", "Texas (TX)"), + ("UM", "United States Minor Outlying Islands (UM)"), + ("UT", "Utah (UT)"), + ("VT", "Vermont (VT)"), + ("VI", "Virgin Islands (VI)"), + ("VA", "Virginia (VA)"), + ("WA", "Washington (WA)"), + ("WV", "West Virginia (WV)"), + ("WI", "Wisconsin (WI)"), + ("WY", "Wyoming (WY)"), + ("AA", "Armed Forces Americas (AA)"), + ("AE", "Armed Forces Africa, Canada, Europe, Middle East (AE)"), + ("AP", "Armed Forces Pacific (AP)"), + ], + max_length=2, + null=True, + verbose_name="state / territory", + ), + ), + ("zipcode", models.CharField(blank=True, max_length=10, null=True, verbose_name="zip code")), + ( + "urbanization", + models.CharField( + blank=True, help_text="Required for Puerto Rico only", null=True, verbose_name="urbanization" + ), + ), + ( + "security_contact_email", + models.EmailField(blank=True, max_length=320, null=True, verbose_name="security contact e-mail"), + ), + ( + "creator", + models.ForeignKey( + help_text="Associated user", + on_delete=django.db.models.deletion.PROTECT, + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "federal_agency", + models.ForeignKey( + default=registrar.models.portfolio.get_default_federal_agency, + help_text="Associated federal agency", + on_delete=django.db.models.deletion.PROTECT, + to="registrar.federalagency", + ), + ), + ], + options={ + "abstract": False, + }, + ), + migrations.AddField( + model_name="domaininformation", + name="portfolio", + field=models.OneToOneField( + blank=True, + help_text="Portfolio associated with this domain", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainRequest_portfolio", + to="registrar.portfolio", + ), + ), + migrations.AddField( + model_name="domainrequest", + name="portfolio", + field=models.OneToOneField( + blank=True, + help_text="Portfolio associated with this domain", + null=True, + on_delete=django.db.models.deletion.PROTECT, + related_name="DomainInformation_portfolio", + to="registrar.portfolio", + ), + ), + ] diff --git a/src/registrar/migrations/0102_create_groups_v13.py b/src/registrar/migrations/0102_create_groups_v13.py new file mode 100644 index 000000000..6ddea8260 --- /dev/null +++ b/src/registrar/migrations/0102_create_groups_v13.py @@ -0,0 +1,37 @@ +# This migration creates the create_full_access_group and create_cisa_analyst_group groups +# It is dependent on 0079 (which populates federal agencies) +# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS +# in the user_group model then: +# [NOT RECOMMENDED] +# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions +# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups +# step 3: fake run the latest migration in the migrations list +# [RECOMMENDED] +# Alternatively: +# step 1: duplicate the migration that loads data +# step 2: docker-compose exec app ./manage.py migrate + +from django.db import migrations +from registrar.models import UserGroup +from typing import Any + + +# For linting: RunPython expects a function reference, +# so let's give it one +def create_groups(apps, schema_editor) -> Any: + UserGroup.create_cisa_analyst_group(apps, schema_editor) + UserGroup.create_full_access_group(apps, schema_editor) + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0101_portfolio_domaininformation_portfolio_and_more"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index f084a5d8b..401b8012d 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -16,6 +16,7 @@ from .website import Website from .transition_domain import TransitionDomain from .verified_by_staff import VerifiedByStaff from .waffle_flag import WaffleFlag +from .portfolio import Portfolio __all__ = [ @@ -36,6 +37,7 @@ __all__ = [ "TransitionDomain", "VerifiedByStaff", "WaffleFlag", + "Portfolio" ] auditlog.register(Contact) @@ -55,3 +57,4 @@ auditlog.register(Website) auditlog.register(TransitionDomain) auditlog.register(VerifiedByStaff) auditlog.register(WaffleFlag) +auditlog.register(Portfolio) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 8235981f5..198bf29c5 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -59,14 +59,14 @@ class DomainInformation(TimeStampedModel): # portfolio - # portfolio = models.OneToOneField( - # "registrar.Portfolio", - # on_delete=models.PROTECT, - # null=True, - # blank=True, - # related_name="DomainRequest_portfolio", - # help_text="Portfolio associated with this domain", - # ) + portfolio = models.OneToOneField( + "registrar.Portfolio", + on_delete=models.PROTECT, + null=True, + blank=True, + related_name="DomainRequest_portfolio", + help_text="Portfolio associated with this domain", + ) domain_request = models.OneToOneField( "registrar.DomainRequest", diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 8c0734ba9..a594f29f5 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -287,14 +287,14 @@ class DomainRequest(TimeStampedModel): ) # portfolio - # portfolio = models.OneToOneField( - # "registrar.Portfolio", - # on_delete=models.PROTECT, - # null=True, - # blank=True, - # related_name="DomainInformation_portfolio", - # help_text="Portfolio associated with this domain", - # ) + portfolio = models.OneToOneField( + "registrar.Portfolio", + on_delete=models.PROTECT, + null=True, + blank=True, + related_name="DomainInformation_portfolio", + help_text="Portfolio associated with this domain", + ) # This is the domain request user who created this domain request. The contact # information that they gave is in the `submitter` field diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index cbd39f3c7..43605b27d 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -3,10 +3,12 @@ from django_fsm import FSMField # type: ignore from registrar.models.domain_request import DomainRequest from registrar.models.federal_agency import FederalAgency -from registrar.models.domain import State from .utility.time_stamped_model import TimeStampedModel +def get_default_federal_agency(): + """returns non-federal agency""" + return FederalAgency.objects.filter(agency="Non-Federal Agency").first() class Portfolio(TimeStampedModel): """ @@ -16,11 +18,10 @@ class Portfolio(TimeStampedModel): # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices - + StateTerritoryChoices = DomainRequest.StateTerritoryChoices # creator- user foreign key- stores who created this model should get the user who is adding # it via django admin if there is a user (aka not done via commandline/ manual means)""" - # TODO: auto-populate creator = models.ForeignKey( "registrar.User", on_delete=models.PROTECT, @@ -40,18 +41,7 @@ class Portfolio(TimeStampedModel): on_delete=models.PROTECT, help_text="Associated federal agency", unique=False, - default=FederalAgency.objects.filter(agency="Non-Federal Agency").first() - ) - - # creator- user foreign key- stores who created this model - # should get the user who is adding it via django admin if there - # is a user (aka not done via commandline/ manual means) - # TODO: auto-populate - creator = models.ForeignKey( - "registrar.User", - on_delete=models.PROTECT, - help_text="Associated user", - unique=False + default=get_default_federal_agency ) # organization type- should match organization types allowed on domain info @@ -64,7 +54,7 @@ class Portfolio(TimeStampedModel): ) # organization name - # TODO: org name will be the same as federal agency, if it is federal, + # NOTE: org name will be the same as federal agency, if it is federal, # otherwise it will be the actual org name. If nothing is entered for # org name and it is a federal organization, have this field fill with # the federal agency text name. @@ -90,17 +80,13 @@ class Portfolio(TimeStampedModel): null=True, blank=True, ) - # state (copied from domain.py -- imports enums from domain.py) - state = FSMField( - max_length=21, - choices=State.choices, - default=State.UNKNOWN, - # cannot change state directly, particularly in Django admin - protected=True, - # This must be defined for custom state help messages, - # as otherwise the view will purge the help field as it does not exist. - help_text=" ", - verbose_name="domain state", + # state (copied from domain_request.py -- imports enums from domain_request.py) + state_territory = models.CharField( + max_length=2, + choices=StateTerritoryChoices.choices, + null=True, + blank=True, + verbose_name="state / territory", ) # zipcode zipcode = models.CharField( @@ -123,55 +109,4 @@ class Portfolio(TimeStampedModel): blank=True, verbose_name="security contact e-mail", max_length=320, - ) - - def save(self, *args, **kwargs): - # Call the parent class's save method to perform the actual save - super().save(*args, **kwargs) - - # TODO: - # ---- auto-populate creator ---- - # creator- user foreign key- stores who created this model - # should get the user who is adding it via django admin if there - # is a user (aka not done via commandline/ manual means) - - # ---- update organization name ---- - # org name will be the same as federal agency, if it is federal, - # otherwise it will be the actual org name. If nothing is entered for - # org name and it is a federal organization, have this field fill with - # the federal agency text name. - is_federal = self.organization_type == self.OrganizationChoices.FEDERAL - if is_federal: - self.organization_name = DomainRequest.OrganizationChoicesVerbose(self.organization_type) - #NOTE: Is this what is meant by "federal agency text name?" - - - # ----------------------------------- - # if self.user: - # updated = False - - # # Update first name and last name if necessary - # if not self.user.first_name or not self.user.last_name: - # self.user.first_name = self.first_name - # self.user.last_name = self.last_name - # updated = True - - # # Update phone if necessary - # if not self.user.phone: - # self.user.phone = self.phone - # updated = True - - # # Save user if any updates were made - # if updated: - # self.user.save() - # ----------------------------------- - - # def __str__(self): - # if self.first_name or self.last_name: - # return self.get_formatted_name() - # elif self.email: - # return self.email - # elif self.pk: - # return str(self.pk) - # else: - # return "" + ) \ No newline at end of file From a15772115be3131ccf905b4452e52226ec83e876 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Wed, 12 Jun 2024 22:36:08 -0600 Subject: [PATCH 07/25] update migration --- .../0101_portfolio_domaininformation_portfolio_and_more.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py index fd2138801..02ff5a1e8 100644 --- a/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-12 22:15 +# Generated by Django 4.2.10 on 2024-06-12 23:06 from django.conf import settings from django.db import migrations, models From 97ee855de6afc3b80c91e3f169d57f7f0cd29c68 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 13 Jun 2024 07:21:06 -0400 Subject: [PATCH 08/25] added FederalAgency, updated for using bulk --- docs/operations/import_export.md | 7 +++++-- src/registrar/admin.py | 14 +++++++++++++- src/registrar/management/commands/clean_tables.py | 6 ++++-- src/registrar/management/commands/export_tables.py | 1 + src/registrar/management/commands/import_tables.py | 8 +++++++- 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/docs/operations/import_export.md b/docs/operations/import_export.md index 4810774e4..b7fd50d52 100644 --- a/docs/operations/import_export.md +++ b/docs/operations/import_export.md @@ -32,6 +32,7 @@ For reference, the zip file will contain the following tables in csv form: * DomainInformation * DomainUserRole * DraftDomain +* FederalAgency * Websites * Host * HostIP @@ -76,13 +77,14 @@ For reference, this deletes all rows from the following tables: * DomainInformation * DomainRequest * Domain -* User (all but the current user) +* User * Contact * Websites * DraftDomain * HostIP * Host * PublicContact +* FederalAgency #### Importing into Target Environment @@ -115,7 +117,7 @@ cf ssh {target-app} example cleaning getgov-backup: cf ssh getgov-backup /tmp/lifecycle/backup -./manage.py import_tables --skipEppSave=False +./manage.py import_tables --no-skipEppSave For reference, this imports tables in the following order: @@ -126,6 +128,7 @@ For reference, this imports tables in the following order: * HostIP * DraftDomain * Websites +* FederalAgency * DomainRequest * DomainInformation * UserDomainRole diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9ddaf03e7..16aba7ec0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2370,6 +2370,9 @@ class PublicContactResource(resources.ModelResource): class Meta: model = models.PublicContact + use_bulk = True + batch_size = 1000 + force_init_instance = True def __init__(self): """Sets global variables for code tidyness""" @@ -2472,11 +2475,20 @@ class VerifiedByStaffAdmin(ListHeaderAdmin): super().save_model(request, obj, form, change) -class FederalAgencyAdmin(ListHeaderAdmin): +class FederalAgencyResource(resources.ModelResource): + """defines how each field in the referenced model should be mapped to the corresponding fields in the + import/export file""" + + class Meta: + model = models.FederalAgency + + +class FederalAgencyAdmin(ListHeaderAdmin, ImportExportModelAdmin): list_display = ["agency"] search_fields = ["agency"] search_help_text = "Search by agency name." ordering = ["agency"] + resource_classes = [FederalAgencyResource] class UserGroupAdmin(AuditedAdmin): diff --git a/src/registrar/management/commands/clean_tables.py b/src/registrar/management/commands/clean_tables.py index fa37c214d..f0c51390b 100644 --- a/src/registrar/management/commands/clean_tables.py +++ b/src/registrar/management/commands/clean_tables.py @@ -28,6 +28,7 @@ class Command(BaseCommand): * DomainInformation * DomainRequest * DraftDomain + * FederalAgency * Host * HostIp * PublicContact @@ -40,14 +41,15 @@ class Command(BaseCommand): table_names = [ "DomainInformation", "DomainRequest", + "FederalAgency", "PublicContact", + "HostIp", + "Host", "Domain", "User", "Contact", "Website", "DraftDomain", - "HostIp", - "Host", ] for table_name in table_names: diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index f927129fe..bd10ea7c5 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -18,6 +18,7 @@ class Command(BaseCommand): "Domain", "DomainRequest", "DomainInformation", + "FederalAgency", "UserDomainRole", "DraftDomain", "Website", diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index cbe3dc6ee..fcefb9b61 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -36,6 +36,7 @@ class Command(BaseCommand): "HostIp", "DraftDomain", "Website", + "FederalAgency", "DomainRequest", "DomainInformation", "UserDomainRole", @@ -83,7 +84,12 @@ class Command(BaseCommand): result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) if result.has_errors(): - logger.error(f"Errors occurred while importing {csv_filename}: {result.row_errors()}") + logger.error(f"Errors occurred while importing {csv_filename}:") + for row_error in result.row_errors(): + row_index = row_error[0] + errors = row_error[1] + for error in errors: + logger.error(f"Row {row_index} - {error.error} - {error.row}") else: logger.info(f"Successfully imported {csv_filename} into {table_name}") From 552e43409640f3abbf838394f39be6fd50426e4d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Thu, 13 Jun 2024 09:42:33 -0400 Subject: [PATCH 09/25] updated scripts to break datasets into max of 10000 rows --- .../management/commands/export_tables.py | 64 ++++++++++++----- .../management/commands/import_tables.py | 70 ++++++++++--------- 2 files changed, 85 insertions(+), 49 deletions(-) diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index bd10ea7c5..8c2cf7dc4 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -1,6 +1,9 @@ +import glob import logging +import math import os import pyzipper +import tablib from django.core.management import BaseCommand import registrar.admin @@ -37,17 +40,21 @@ class Command(BaseCommand): zip_filename = "tmp/exported_tables.zip" with pyzipper.AESZipFile(zip_filename, "w", compression=pyzipper.ZIP_DEFLATED) as zipf: for table_name in table_names: - csv_filename = f"tmp/{table_name}.csv" - if os.path.exists(csv_filename): - zipf.write(csv_filename, os.path.basename(csv_filename)) - logger.info(f"Added {csv_filename} to zip archive {zip_filename}") - # Remove the CSV files after adding them to the zip file - for table_name in table_names: - csv_filename = f"tmp/{table_name}.csv" - if os.path.exists(csv_filename): - os.remove(csv_filename) - logger.info(f"Removed temporary file {csv_filename}") + # Define the directory and the pattern + tmp_dir = 'tmp' + pattern = os.path.join(tmp_dir, f'{table_name}_*.csv') + zip_file_path = os.path.join(tmp_dir, 'exported_files.zip') + + # Find all files that match the pattern + for file_path in glob.glob(pattern): + # Add each file to the zip archive + zipf.write(file_path, os.path.basename(file_path)) + logger.info(f'Added {file_path} to {zip_file_path}') + + # Remove the file after adding to zip + os.remove(file_path) + logger.info(f'Removed {file_path}') def export_table(self, table_name): """Export a given table to a csv file in the tmp directory""" @@ -55,11 +62,36 @@ class Command(BaseCommand): try: resourceclass = getattr(registrar.admin, resourcename) dataset = resourceclass().export() - filename = f"tmp/{table_name}.csv" - with open(filename, "w") as outputfile: - outputfile.write(dataset.csv) - logger.info(f"Successfully exported {table_name} to {filename}") - except AttributeError: - logger.error(f"Resource class {resourcename} not found in registrar.admin") + if not isinstance(dataset, tablib.Dataset): + raise ValueError(f"Exported data from {resourcename} is not a tablib.Dataset") + + # Determine the number of rows per file + rows_per_file = 10000 + total_rows = len(dataset) + + # Calculate the number of files needed + num_files = math.ceil(total_rows / rows_per_file) + logger.info(f'splitting {table_name} into {num_files} files') + + # Split the dataset and export each chunk to a separate file + for i in range(num_files): + start_row = i * rows_per_file + end_row = start_row + rows_per_file + + # Create a new dataset for the chunk + chunk = tablib.Dataset(headers=dataset.headers) + for row in dataset[start_row:end_row]: + chunk.append(row) + #chunk = dataset[start_row:end_row] + + # Export the chunk to a new file + filename = f'tmp/{table_name}_{i + 1}.csv' + with open(filename, 'w') as f: + f.write(chunk.export('csv')) + + logger.info(f'Successfully exported {table_name} into {num_files} files.') + + except AttributeError as ae: + logger.error(f"Resource class {resourcename} not found in registrar.admin: {ae}") except Exception as e: logger.error(f"Failed to export {table_name}: {e}") diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index fcefb9b61..d04d0dbb2 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -1,4 +1,5 @@ import argparse +import glob import logging import os import pyzipper @@ -64,43 +65,46 @@ class Command(BaseCommand): """Import data from a CSV file into the given table""" resourcename = f"{table_name}Resource" - csv_filename = f"tmp/{table_name}.csv" - try: - if not os.path.exists(csv_filename): - logger.error(f"CSV file {csv_filename} not found.") - return - # if table_name is Contact, clean the table first - # User table is loaded before Contact, and signals create - # rows in Contact table which break the import, so need - # to be cleaned again before running import on Contact table - if table_name == "Contact": - self.clean_table(table_name) + # if table_name is Contact, clean the table first + # User table is loaded before Contact, and signals create + # rows in Contact table which break the import, so need + # to be cleaned again before running import on Contact table + if table_name == "Contact": + self.clean_table(table_name) - resourceclass = getattr(registrar.admin, resourcename) - resource_instance = resourceclass() - with open(csv_filename, "r") as csvfile: - dataset = tablib.Dataset().load(csvfile.read(), format="csv") - result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) + # Define the directory and the pattern for csv filenames + tmp_dir = 'tmp' + pattern = os.path.join(tmp_dir, f'{table_name}_*.csv') - if result.has_errors(): - logger.error(f"Errors occurred while importing {csv_filename}:") - for row_error in result.row_errors(): - row_index = row_error[0] - errors = row_error[1] - for error in errors: - logger.error(f"Row {row_index} - {error.error} - {error.row}") - else: - logger.info(f"Successfully imported {csv_filename} into {table_name}") + resourceclass = getattr(registrar.admin, resourcename) + resource_instance = resourceclass() - except AttributeError: - logger.error(f"Resource class {resourcename} not found in registrar.admin") - except Exception as e: - logger.error(f"Failed to import {csv_filename}: {e}") - finally: - if os.path.exists(csv_filename): - os.remove(csv_filename) - logger.info(f"Removed temporary file {csv_filename}") + # Find all files that match the pattern + for csv_filename in glob.glob(pattern): + try: + with open(csv_filename, "r") as csvfile: + dataset = tablib.Dataset().load(csvfile.read(), format="csv") + result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) + + if result.has_errors(): + logger.error(f"Errors occurred while importing {csv_filename}:") + for row_error in result.row_errors(): + row_index = row_error[0] + errors = row_error[1] + for error in errors: + logger.error(f"Row {row_index} - {error.error} - {error.row}") + else: + logger.info(f"Successfully imported {csv_filename} into {table_name}") + + except AttributeError: + logger.error(f"Resource class {resourcename} not found in registrar.admin") + except Exception as e: + logger.error(f"Failed to import {csv_filename}: {e}") + finally: + if os.path.exists(csv_filename): + os.remove(csv_filename) + logger.info(f"Removed temporary file {csv_filename}") def clean_table(self, table_name): """Delete all rows in the given table""" From 2a90335499c052e7d259fd2ca9e57c2d2b081991 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 13 Jun 2024 16:32:45 -0600 Subject: [PATCH 10/25] updated migrations --- ...=> 0102_portfolio_domaininformation_portfolio_and_more.py} | 4 ++-- .../{0102_create_groups_v13.py => 0103_create_groups_v13.py} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/registrar/migrations/{0101_portfolio_domaininformation_portfolio_and_more.py => 0102_portfolio_domaininformation_portfolio_and_more.py} (98%) rename src/registrar/migrations/{0102_create_groups_v13.py => 0103_create_groups_v13.py} (95%) diff --git a/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py similarity index 98% rename from src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py rename to src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py index 02ff5a1e8..3882e503a 100644 --- a/src/registrar/migrations/0101_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-12 23:06 +# Generated by Django 4.2.10 on 2024-06-13 22:29 from django.conf import settings from django.db import migrations, models @@ -9,7 +9,7 @@ import registrar.models.portfolio class Migration(migrations.Migration): dependencies = [ - ("registrar", "0100_domainrequest_action_needed_reason"), + ("registrar", "0101_domaininformation_cisa_representative_first_name_and_more"), ] operations = [ diff --git a/src/registrar/migrations/0102_create_groups_v13.py b/src/registrar/migrations/0103_create_groups_v13.py similarity index 95% rename from src/registrar/migrations/0102_create_groups_v13.py rename to src/registrar/migrations/0103_create_groups_v13.py index 6ddea8260..7504ff331 100644 --- a/src/registrar/migrations/0102_create_groups_v13.py +++ b/src/registrar/migrations/0103_create_groups_v13.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0101_portfolio_domaininformation_portfolio_and_more"), + ("registrar", "0102_portfolio_domaininformation_portfolio_and_more"), ] operations = [ From 1dfc68ce47f36b1ae02789430c2d2175fd29a84a Mon Sep 17 00:00:00 2001 From: CocoByte Date: Thu, 13 Jun 2024 16:39:17 -0600 Subject: [PATCH 11/25] linted --- src/registrar/admin.py | 3 +-- src/registrar/models/__init__.py | 2 +- src/registrar/models/domain_information.py | 3 +-- src/registrar/models/domain_request.py | 2 +- src/registrar/models/portfolio.py | 23 ++++++++++------------ 5 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 70b06abb6..9abb909d2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2490,12 +2490,11 @@ class PortfolioAdmin(ListHeaderAdmin): is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL if is_federal: obj.organization_name = obj.organization_type - #NOTE: What is meant by "federal agency text name?" + # NOTE: What is meant by "federal agency text name?" super().save_model(request, obj, form, change) - class FederalAgencyAdmin(ListHeaderAdmin): list_display = ["agency"] search_fields = ["agency"] diff --git a/src/registrar/models/__init__.py b/src/registrar/models/__init__.py index 401b8012d..b2cffaf32 100644 --- a/src/registrar/models/__init__.py +++ b/src/registrar/models/__init__.py @@ -37,7 +37,7 @@ __all__ = [ "TransitionDomain", "VerifiedByStaff", "WaffleFlag", - "Portfolio" + "Portfolio", ] auditlog.register(Contact) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index c76c6fe3e..c058ab209 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -57,7 +57,6 @@ class DomainInformation(TimeStampedModel): help_text="Person who submitted the domain request", ) - # portfolio portfolio = models.OneToOneField( "registrar.Portfolio", @@ -66,7 +65,7 @@ class DomainInformation(TimeStampedModel): blank=True, related_name="DomainRequest_portfolio", help_text="Portfolio associated with this domain", - ) + ) domain_request = models.OneToOneField( "registrar.DomainRequest", diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 75fe0dc0b..3e616f202 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -307,7 +307,7 @@ class DomainRequest(TimeStampedModel): blank=True, related_name="DomainInformation_portfolio", help_text="Portfolio associated with this domain", - ) + ) # This is the domain request user who created this domain request. The contact # information that they gave is in the `submitter` field diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 43605b27d..ca5365177 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -1,47 +1,44 @@ from django.db import models -from django_fsm import FSMField # type: ignore from registrar.models.domain_request import DomainRequest from registrar.models.federal_agency import FederalAgency from .utility.time_stamped_model import TimeStampedModel + def get_default_federal_agency(): """returns non-federal agency""" return FederalAgency.objects.filter(agency="Non-Federal Agency").first() + class Portfolio(TimeStampedModel): """ Portfolio is used for organizing domains/domain-requests into manageable groups. """ - + # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices - # creator- user foreign key- stores who created this model should get the user who is adding + # creator- user foreign key- stores who created this model should get the user who is adding # it via django admin if there is a user (aka not done via commandline/ manual means)""" - creator = models.ForeignKey( - "registrar.User", - on_delete=models.PROTECT, - help_text="Associated user", - unique=False - ) + creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", unique=False) # notes- text field (copy what is done on requests/domains) notes = models.TextField( null=True, blank=True, ) - - # federal agency - FK to fed agency table (Not nullable, should default to the Non-federal agency value in the fed agency table) + + # federal agency - FK to fed agency table (Not nullable, should default + # to the Non-federal agency value in the fed agency table) federal_agency = models.ForeignKey( "registrar.FederalAgency", on_delete=models.PROTECT, help_text="Associated federal agency", unique=False, - default=get_default_federal_agency + default=get_default_federal_agency, ) # organization type- should match organization types allowed on domain info @@ -109,4 +106,4 @@ class Portfolio(TimeStampedModel): blank=True, verbose_name="security contact e-mail", max_length=320, - ) \ No newline at end of file + ) From aa0a398c06509b21e7d0cdfda2f8afeadfc4f0c9 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Jun 2024 12:17:08 -0400 Subject: [PATCH 12/25] updated unit tests for the associated changes in this PR --- src/registrar/admin.py | 13 +- .../management/commands/export_tables.py | 31 ++-- .../management/commands/import_tables.py | 11 +- .../tests/test_management_scripts.py | 148 +++++++++--------- 4 files changed, 101 insertions(+), 102 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 16aba7ec0..46b468eb2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2370,14 +2370,15 @@ class PublicContactResource(resources.ModelResource): class Meta: model = models.PublicContact - use_bulk = True - batch_size = 1000 - force_init_instance = True + # may want to consider these bulk options in future, so left in as comments + # use_bulk = True + # batch_size = 1000 + # force_init_instance = True def __init__(self): """Sets global variables for code tidyness""" super().__init__() - self.skip_epp_save=False + self.skip_epp_save = False def import_data( self, @@ -2387,10 +2388,10 @@ class PublicContactResource(resources.ModelResource): use_transactions=None, collect_failed_rows=False, rollback_on_validation_errors=False, - **kwargs + **kwargs, ): """Override import_data to set self.skip_epp_save if in kwargs""" - self.skip_epp_save = kwargs.get('skip_epp_save', False) + self.skip_epp_save = kwargs.get("skip_epp_save", False) return super().import_data( dataset, dry_run, diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index 8c2cf7dc4..e5c940a40 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -41,20 +41,21 @@ class Command(BaseCommand): with pyzipper.AESZipFile(zip_filename, "w", compression=pyzipper.ZIP_DEFLATED) as zipf: for table_name in table_names: - # Define the directory and the pattern - tmp_dir = 'tmp' - pattern = os.path.join(tmp_dir, f'{table_name}_*.csv') - zip_file_path = os.path.join(tmp_dir, 'exported_files.zip') + # Define the tmp directory and the file pattern + tmp_dir = "tmp" + pattern = f"{table_name}_" + zip_file_path = os.path.join(tmp_dir, "exported_files.zip") # Find all files that match the pattern - for file_path in glob.glob(pattern): + matching_files = [file for file in os.listdir(tmp_dir) if file.startswith(pattern)] + for file_path in matching_files: # Add each file to the zip archive zipf.write(file_path, os.path.basename(file_path)) - logger.info(f'Added {file_path} to {zip_file_path}') - + logger.info(f"Added {file_path} to {zip_file_path}") + # Remove the file after adding to zip os.remove(file_path) - logger.info(f'Removed {file_path}') + logger.info(f"Removed {file_path}") def export_table(self, table_name): """Export a given table to a csv file in the tmp directory""" @@ -71,7 +72,8 @@ class Command(BaseCommand): # Calculate the number of files needed num_files = math.ceil(total_rows / rows_per_file) - logger.info(f'splitting {table_name} into {num_files} files') + + logger.info(f"splitting {table_name} into {num_files} files") # Split the dataset and export each chunk to a separate file for i in range(num_files): @@ -82,16 +84,15 @@ class Command(BaseCommand): chunk = tablib.Dataset(headers=dataset.headers) for row in dataset[start_row:end_row]: chunk.append(row) - #chunk = dataset[start_row:end_row] # Export the chunk to a new file - filename = f'tmp/{table_name}_{i + 1}.csv' - with open(filename, 'w') as f: - f.write(chunk.export('csv')) + filename = f"tmp/{table_name}_{i + 1}.csv" + with open(filename, "w") as f: + f.write(chunk.export("csv")) - logger.info(f'Successfully exported {table_name} into {num_files} files.') + logger.info(f"Successfully exported {table_name} into {num_files} files.") except AttributeError as ae: - logger.error(f"Resource class {resourcename} not found in registrar.admin: {ae}") + logger.error(f"Resource class {resourcename} not found in registrar.admin") except Exception as e: logger.error(f"Failed to export {table_name}: {e}") diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index d04d0dbb2..abe26830f 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -18,7 +18,7 @@ class Command(BaseCommand): def add_arguments(self, parser): """Add command line arguments.""" - parser.add_argument('--skipEppSave', default=True, action=argparse.BooleanOptionalAction) + parser.add_argument("--skipEppSave", default=True, action=argparse.BooleanOptionalAction) def handle(self, **options): """Extracts CSV files from a zip archive and imports them into the respective tables""" @@ -74,19 +74,20 @@ class Command(BaseCommand): self.clean_table(table_name) # Define the directory and the pattern for csv filenames - tmp_dir = 'tmp' - pattern = os.path.join(tmp_dir, f'{table_name}_*.csv') + tmp_dir = "tmp" + #pattern = os.path.join(tmp_dir, f"{table_name}_*.csv") + pattern = f"{table_name}_" resourceclass = getattr(registrar.admin, resourcename) resource_instance = resourceclass() # Find all files that match the pattern - for csv_filename in glob.glob(pattern): + matching_files = [file for file in os.listdir(tmp_dir) if file.startswith(pattern)] + for csv_filename in matching_files: try: with open(csv_filename, "r") as csvfile: dataset = tablib.Dataset().load(csvfile.read(), format="csv") result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) - if result.has_errors(): logger.error(f"Errors occurred while importing {csv_filename}:") for row_error in result.row_errors(): diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 500953f02..006a39231 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -7,6 +7,7 @@ from django.utils.module_loading import import_string import logging import pyzipper from registrar.management.commands.clean_tables import Command as CleanTablesCommand +from registrar.management.commands.export_tables import Command as ExportTablesCommand from registrar.models import ( User, Domain, @@ -869,88 +870,77 @@ class TestCleanTables(TestCase): self.logger_mock.error.assert_any_call("Error cleaning table DomainInformation: Some error") + + class TestExportTables(MockEppLib): """Test the export_tables script""" def setUp(self): + self.command = ExportTablesCommand() self.logger_patcher = patch("registrar.management.commands.export_tables.logger") self.logger_mock = self.logger_patcher.start() def tearDown(self): self.logger_patcher.stop() - @patch("registrar.management.commands.export_tables.os.makedirs") - @patch("registrar.management.commands.export_tables.os.path.exists") - @patch("registrar.management.commands.export_tables.os.remove") - @patch("registrar.management.commands.export_tables.pyzipper.AESZipFile") + @patch("os.makedirs") + @patch("os.path.exists") + @patch("os.remove") + @patch("pyzipper.AESZipFile") @patch("registrar.management.commands.export_tables.getattr") - @patch("builtins.open", new_callable=mock_open, read_data=b"mock_csv_data") - @patch("django.utils.translation.trans_real._translations", {}) - @patch("django.utils.translation.trans_real.translation") + @patch("builtins.open", new_callable=mock_open) + @patch("os.listdir") def test_handle( - self, mock_translation, mock_file, mock_getattr, mock_zipfile, mock_remove, mock_path_exists, mock_makedirs + self, mock_listdir, mock_open, mock_getattr, mock_zipfile, mock_remove, mock_path_exists, mock_makedirs ): """test that the handle method properly exports tables""" - with less_console_noise(): - # Mock os.makedirs to do nothing - mock_makedirs.return_value = None + # Mock os.makedirs to do nothing + mock_makedirs.return_value = None - # Mock os.path.exists to always return True - mock_path_exists.return_value = True + # Mock os.path.exists to always return True + mock_path_exists.return_value = True - # Mock the resource class and its export method - mock_resource_class = MagicMock() - mock_dataset = MagicMock() - mock_dataset.csv = b"mock_csv_data" - mock_resource_class().export.return_value = mock_dataset - mock_getattr.return_value = mock_resource_class + # Check that the export_table function was called for each table + table_names = [ + "User", "Contact", "Domain", "DomainRequest", "DomainInformation", "FederalAgency", + "UserDomainRole", "DraftDomain", "Website", "HostIp", "Host", "PublicContact", + ] - # Mock translation function to return a dummy translation object - mock_translation.return_value = MagicMock() + # Mock directory listing + mock_listdir.side_effect = lambda path: [f"{table}_1.csv" for table in table_names] - call_command("export_tables") + # Mock the resource class and its export method + mock_dataset = tablib.Dataset() + mock_dataset.headers = ["header1", "header2"] + mock_dataset.append(["row1_col1", "row1_col2"]) + mock_resource_class = MagicMock() + mock_resource_class().export.return_value = mock_dataset + mock_getattr.return_value = mock_resource_class - # Check that os.makedirs was called once to create the tmp directory - mock_makedirs.assert_called_once_with("tmp", exist_ok=True) + command_instance = ExportTablesCommand() + command_instance.handle() - # Check that the export_table function was called for each table - table_names = [ - "User", - "Contact", - "Domain", - "DomainRequest", - "DomainInformation", - "UserDomainRole", - "DraftDomain", - "Website", - "HostIp", - "Host", - "PublicContact", - ] + # Check that os.makedirs was called once to create the tmp directory + mock_makedirs.assert_called_once_with("tmp", exist_ok=True) - # Check that the CSV file was written - for table_name in table_names: - mock_file().write.assert_any_call(b"mock_csv_data") - # Check that os.path.exists was called - mock_path_exists.assert_any_call(f"tmp/{table_name}.csv") - # Check that os.remove was called - mock_remove.assert_any_call(f"tmp/{table_name}.csv") + # Check that the CSV file was written + for table_name in table_names: + # Check that os.remove was called + mock_remove.assert_any_call(f"{table_name}_1.csv") - # Check that the zipfile was created and files were added - mock_zipfile.assert_called_once_with("tmp/exported_tables.zip", "w", compression=pyzipper.ZIP_DEFLATED) - zipfile_instance = mock_zipfile.return_value.__enter__.return_value - for table_name in table_names: - zipfile_instance.write.assert_any_call(f"tmp/{table_name}.csv", f"{table_name}.csv") + # Check that the zipfile was created and files were added + mock_zipfile.assert_called_once_with("tmp/exported_tables.zip", "w", compression=pyzipper.ZIP_DEFLATED) + zipfile_instance = mock_zipfile.return_value.__enter__.return_value + for table_name in table_names: + zipfile_instance.write.assert_any_call(f"{table_name}_1.csv", f"{table_name}_1.csv") - # Verify logging for added files - for table_name in table_names: - self.logger_mock.info.assert_any_call( - f"Added tmp/{table_name}.csv to zip archive tmp/exported_tables.zip" - ) + # Verify logging for added files + for table_name in table_names: + self.logger_mock.info.assert_any_call(f"Added {table_name}_1.csv to tmp/exported_files.zip") - # Verify logging for removed files - for table_name in table_names: - self.logger_mock.info.assert_any_call(f"Removed temporary file tmp/{table_name}.csv") + # Verify logging for removed files + for table_name in table_names: + self.logger_mock.info.assert_any_call(f"Removed {table_name}_1.csv") @patch("registrar.management.commands.export_tables.getattr") def test_export_table_handles_missing_resource_class(self, mock_getattr): @@ -995,8 +985,10 @@ class TestImportTables(TestCase): @patch("registrar.management.commands.import_tables.logger") @patch("registrar.management.commands.import_tables.getattr") @patch("django.apps.apps.get_model") + @patch("os.listdir") def test_handle( self, + mock_listdir, mock_get_model, mock_getattr, mock_logger, @@ -1019,6 +1011,24 @@ class TestImportTables(TestCase): mock_zipfile_instance = mock_zipfile.return_value.__enter__.return_value mock_zipfile_instance.extractall.return_value = None + # Check that the import_table function was called for each table + table_names = [ + "User", + "Contact", + "Domain", + "DomainRequest", + "DomainInformation", + "UserDomainRole", + "DraftDomain", + "Website", + "HostIp", + "Host", + "PublicContact", + ] + + # Mock directory listing + mock_listdir.side_effect = lambda path: [f"{table}_1.csv" for table in table_names] + # Mock the CSV file content csv_content = b"mock_csv_data" @@ -1054,23 +1064,9 @@ class TestImportTables(TestCase): # Check that extractall was called once to extract the zip file contents mock_zipfile_instance.extractall.assert_called_once_with("tmp") - # Check that the import_table function was called for each table - table_names = [ - "User", - "Contact", - "Domain", - "DomainRequest", - "DomainInformation", - "UserDomainRole", - "DraftDomain", - "Website", - "HostIp", - "Host", - "PublicContact", - ] # Check that os.path.exists was called for each table for table_name in table_names: - mock_path_exists.assert_any_call(f"tmp/{table_name}.csv") + mock_path_exists.assert_any_call(f"{table_name}_1.csv") # Check that clean_tables is called for Contact mock_get_model.assert_any_call("registrar", "Contact") @@ -1079,18 +1075,18 @@ class TestImportTables(TestCase): # Check that logger.info was called for each successful import for table_name in table_names: - mock_logger.info.assert_any_call(f"Successfully imported tmp/{table_name}.csv into {table_name}") + mock_logger.info.assert_any_call(f"Successfully imported {table_name}_1.csv into {table_name}") # Check that logger.error was not called for resource class not found mock_logger.error.assert_not_called() # Check that os.remove was called for each CSV file for table_name in table_names: - mock_remove.assert_any_call(f"tmp/{table_name}.csv") + mock_remove.assert_any_call(f"{table_name}_1.csv") # Check that logger.info was called for each CSV file removal for table_name in table_names: - mock_logger.info.assert_any_call(f"Removed temporary file tmp/{table_name}.csv") + mock_logger.info.assert_any_call(f"Removed temporary file {table_name}_1.csv") @patch("registrar.management.commands.import_tables.logger") @patch("registrar.management.commands.import_tables.os.makedirs") From 38f7ef50dbce9bbfaac2fde9a029e71fb0bb92ea Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Jun 2024 12:32:17 -0400 Subject: [PATCH 13/25] formatted for code readability --- .../management/commands/export_tables.py | 3 +-- .../management/commands/import_tables.py | 2 -- src/registrar/tests/test_management_scripts.py | 18 +++++++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index e5c940a40..d9b45ec94 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -1,4 +1,3 @@ -import glob import logging import math import os @@ -92,7 +91,7 @@ class Command(BaseCommand): logger.info(f"Successfully exported {table_name} into {num_files} files.") - except AttributeError as ae: + except AttributeError: logger.error(f"Resource class {resourcename} not found in registrar.admin") except Exception as e: logger.error(f"Failed to export {table_name}: {e}") diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index abe26830f..7797d8d75 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -1,5 +1,4 @@ import argparse -import glob import logging import os import pyzipper @@ -75,7 +74,6 @@ class Command(BaseCommand): # Define the directory and the pattern for csv filenames tmp_dir = "tmp" - #pattern = os.path.join(tmp_dir, f"{table_name}_*.csv") pattern = f"{table_name}_" resourceclass = getattr(registrar.admin, resourcename) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 006a39231..8e3917d40 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -870,8 +870,6 @@ class TestCleanTables(TestCase): self.logger_mock.error.assert_any_call("Error cleaning table DomainInformation: Some error") - - class TestExportTables(MockEppLib): """Test the export_tables script""" @@ -902,8 +900,18 @@ class TestExportTables(MockEppLib): # Check that the export_table function was called for each table table_names = [ - "User", "Contact", "Domain", "DomainRequest", "DomainInformation", "FederalAgency", - "UserDomainRole", "DraftDomain", "Website", "HostIp", "Host", "PublicContact", + "User", + "Contact", + "Domain", + "DomainRequest", + "DomainInformation", + "FederalAgency", + "UserDomainRole", + "DraftDomain", + "Website", + "HostIp", + "Host", + "PublicContact", ] # Mock directory listing @@ -1025,7 +1033,7 @@ class TestImportTables(TestCase): "Host", "PublicContact", ] - + # Mock directory listing mock_listdir.side_effect = lambda path: [f"{table}_1.csv" for table in table_names] From 41bfa3991d45ab07e48c9216d6b4a09f5aa3e613 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Jun 2024 12:48:20 -0400 Subject: [PATCH 14/25] code cleanup --- src/registrar/admin.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 46b468eb2..5344d7059 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2402,19 +2402,6 @@ class PublicContactResource(resources.ModelResource): **kwargs, ) - # def import_row(self, row, instance_loader, using_transactions=True, dry_run=False, raise_errors=None, **kwargs): - # """Override kwargs skip_epp_save and set to True""" - # kwargs["skip_epp_save"] = True - # super().import_data() - # return super().import_row( - # row, - # instance_loader, - # using_transactions=using_transactions, - # dry_run=dry_run, - # raise_errors=raise_errors, - # **kwargs, - # ) - def save_instance(self, instance, is_create, using_transactions=True, dry_run=False): """Override save_instance setting skip_epp_save to True""" self.before_save_instance(instance, using_transactions, dry_run) From 08b0b592b27fd124f0dd25265b0a79f8de4d92be Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Fri, 14 Jun 2024 15:17:57 -0400 Subject: [PATCH 15/25] fixed paths on export and import scripts --- src/registrar/management/commands/export_tables.py | 4 ++-- src/registrar/management/commands/import_tables.py | 2 +- src/registrar/tests/test_management_scripts.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index d9b45ec94..7d7521c1f 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -49,11 +49,11 @@ class Command(BaseCommand): matching_files = [file for file in os.listdir(tmp_dir) if file.startswith(pattern)] for file_path in matching_files: # Add each file to the zip archive - zipf.write(file_path, os.path.basename(file_path)) + zipf.write(f"tmp/{file_path}", os.path.basename(file_path)) logger.info(f"Added {file_path} to {zip_file_path}") # Remove the file after adding to zip - os.remove(file_path) + os.remove(f"tmp/{file_path}") logger.info(f"Removed {file_path}") def export_table(self, table_name): diff --git a/src/registrar/management/commands/import_tables.py b/src/registrar/management/commands/import_tables.py index 7797d8d75..cb78e13bd 100644 --- a/src/registrar/management/commands/import_tables.py +++ b/src/registrar/management/commands/import_tables.py @@ -83,7 +83,7 @@ class Command(BaseCommand): matching_files = [file for file in os.listdir(tmp_dir) if file.startswith(pattern)] for csv_filename in matching_files: try: - with open(csv_filename, "r") as csvfile: + with open(f"tmp/{csv_filename}", "r") as csvfile: dataset = tablib.Dataset().load(csvfile.read(), format="csv") result = resource_instance.import_data(dataset, dry_run=False, skip_epp_save=self.skip_epp_save) if result.has_errors(): diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 8e3917d40..784fe3b67 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -934,13 +934,13 @@ class TestExportTables(MockEppLib): # Check that the CSV file was written for table_name in table_names: # Check that os.remove was called - mock_remove.assert_any_call(f"{table_name}_1.csv") + mock_remove.assert_any_call(f"tmp/{table_name}_1.csv") # Check that the zipfile was created and files were added mock_zipfile.assert_called_once_with("tmp/exported_tables.zip", "w", compression=pyzipper.ZIP_DEFLATED) zipfile_instance = mock_zipfile.return_value.__enter__.return_value for table_name in table_names: - zipfile_instance.write.assert_any_call(f"{table_name}_1.csv", f"{table_name}_1.csv") + zipfile_instance.write.assert_any_call(f"tmp/{table_name}_1.csv", f"{table_name}_1.csv") # Verify logging for added files for table_name in table_names: From 1c96c9ebc96fcfbd7273a6369ccaa274766dbfce Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 17 Jun 2024 11:06:18 -0600 Subject: [PATCH 16/25] fixed failing unit test --- src/registrar/tests/test_admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index e567f9329..7c435c5b0 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2300,6 +2300,7 @@ class TestDomainRequestAdmin(MockEppLib): "rejection_reason", "action_needed_reason", "federal_agency", + 'portfolio', "creator", "investigator", "generic_org_type", From 83243ad4882bfe23cdd6a4a113c2a7a2642c1606 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 17 Jun 2024 11:09:17 -0600 Subject: [PATCH 17/25] fix migrations --- ...=> 0103_portfolio_domaininformation_portfolio_and_more.py} | 4 ++-- .../{0103_create_groups_v13.py => 0104_create_groups_v13.py} | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) rename src/registrar/migrations/{0102_portfolio_domaininformation_portfolio_and_more.py => 0103_portfolio_domaininformation_portfolio_and_more.py} (98%) rename src/registrar/migrations/{0103_create_groups_v13.py => 0104_create_groups_v13.py} (95%) diff --git a/src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py similarity index 98% rename from src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py rename to src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py index 3882e503a..15cec0b73 100644 --- a/src/registrar/migrations/0102_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-13 22:29 +# Generated by Django 4.2.10 on 2024-06-17 17:08 from django.conf import settings from django.db import migrations, models @@ -9,7 +9,7 @@ import registrar.models.portfolio class Migration(migrations.Migration): dependencies = [ - ("registrar", "0101_domaininformation_cisa_representative_first_name_and_more"), + ("registrar", "0102_domain_dsdata_last_change"), ] operations = [ diff --git a/src/registrar/migrations/0103_create_groups_v13.py b/src/registrar/migrations/0104_create_groups_v13.py similarity index 95% rename from src/registrar/migrations/0103_create_groups_v13.py rename to src/registrar/migrations/0104_create_groups_v13.py index 7504ff331..0ce3bafa5 100644 --- a/src/registrar/migrations/0103_create_groups_v13.py +++ b/src/registrar/migrations/0104_create_groups_v13.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0102_portfolio_domaininformation_portfolio_and_more"), + ("registrar", "0103_portfolio_domaininformation_portfolio_and_more"), ] operations = [ From 6b669df5e7254bb3a9a192bbddfcbf334668481e Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 17 Jun 2024 13:50:41 -0600 Subject: [PATCH 18/25] minor cleanup --- ...0103_portfolio_domaininformation_portfolio_and_more.py | 4 ++-- src/registrar/models/domain_request.py | 2 +- src/registrar/models/portfolio.py | 8 ++++---- src/registrar/tests/test_admin.py | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py index 15cec0b73..3edc34b2a 100644 --- a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-17 17:08 +# Generated by Django 4.2.10 on 2024-06-17 19:47 from django.conf import settings from django.db import migrations, models @@ -164,7 +164,7 @@ class Migration(migrations.Migration): name="portfolio", field=models.OneToOneField( blank=True, - help_text="Portfolio associated with this domain", + help_text="Portfolio associated with this domain request", null=True, on_delete=django.db.models.deletion.PROTECT, related_name="DomainInformation_portfolio", diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index cffe5f86c..dfb57cee0 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -310,7 +310,7 @@ class DomainRequest(TimeStampedModel): null=True, blank=True, related_name="DomainInformation_portfolio", - help_text="Portfolio associated with this domain", + help_text="Portfolio associated with this domain request", ) # This is the domain request user who created this domain request. The contact diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index ca5365177..14de445c2 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -21,11 +21,11 @@ class Portfolio(TimeStampedModel): OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices - # creator- user foreign key- stores who created this model should get the user who is adding - # it via django admin if there is a user (aka not done via commandline/ manual means)""" + # creator - stores who created this model. If no creator is specified in DJA, + # then the creator will default to the current request user""" creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", unique=False) - # notes- text field (copy what is done on requests/domains) + # notes - text field (copies what is done on domain requests) notes = models.TextField( null=True, blank=True, @@ -41,7 +41,7 @@ class Portfolio(TimeStampedModel): default=get_default_federal_agency, ) - # organization type- should match organization types allowed on domain info + # organization type - should match organization types allowed on domain info organization_type = models.CharField( max_length=255, choices=OrganizationChoices.choices, diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 62b7bb9d8..802974b6e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -2291,7 +2291,7 @@ class TestDomainRequestAdmin(MockEppLib): "rejection_reason", "action_needed_reason", "federal_agency", - 'portfolio', + "portfolio", "creator", "investigator", "generic_org_type", From 2c96eebc6f69060aea58c71a1658800a9d9e53bd Mon Sep 17 00:00:00 2001 From: CuriousX Date: Mon, 17 Jun 2024 20:57:57 -0600 Subject: [PATCH 19/25] Update src/registrar/admin.py Co-authored-by: zandercymatics <141044360+zandercymatics@users.noreply.github.com> --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index bb256f742..226527e28 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2572,8 +2572,8 @@ class PortfolioAdmin(ListHeaderAdmin): # org name and it is a federal organization, have this field fill with # the federal agency text name. is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL - if is_federal: - obj.organization_name = obj.organization_type + if is_federal and obj.organization_name is None: + obj.organization_name = obj.federal_agency.agency # NOTE: What is meant by "federal agency text name?" super().save_model(request, obj, form, change) From 42efc7372283e1200235b0cc651b444e8b6dfd75 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Mon, 17 Jun 2024 21:20:05 -0600 Subject: [PATCH 20/25] Model updates and cleanup of comments --- src/registrar/admin.py | 8 ++++--- ...io_domaininformation_portfolio_and_more.py | 6 ++--- src/registrar/models/domain_information.py | 2 +- src/registrar/models/domain_request.py | 2 +- src/registrar/models/portfolio.py | 24 ++++++------------- 5 files changed, 17 insertions(+), 25 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 226527e28..25f2afe34 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2562,9 +2562,11 @@ class PortfolioAdmin(ListHeaderAdmin): # ] def save_model(self, request, obj, form, change): - # ---- update creator ---- - # Set the creator field to the current admin user - obj.creator = request.user if request.user.is_authenticated else None + + if not obj.creator is None: + # ---- update creator ---- + # Set the creator field to the current admin user + obj.creator = request.user if request.user.is_authenticated else None # ---- update organization name ---- # org name will be the same as federal agency, if it is federal, diff --git a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py index 3edc34b2a..df0945712 100644 --- a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py @@ -1,4 +1,4 @@ -# Generated by Django 4.2.10 on 2024-06-17 19:47 +# Generated by Django 4.2.10 on 2024-06-18 03:19 from django.conf import settings from django.db import migrations, models @@ -150,7 +150,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name="domaininformation", name="portfolio", - field=models.OneToOneField( + field=models.ForeignKey( blank=True, help_text="Portfolio associated with this domain", null=True, @@ -162,7 +162,7 @@ class Migration(migrations.Migration): migrations.AddField( model_name="domainrequest", name="portfolio", - field=models.OneToOneField( + field=models.ForeignKey( blank=True, help_text="Portfolio associated with this domain request", null=True, diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index c058ab209..b6f2dd9a7 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -58,7 +58,7 @@ class DomainInformation(TimeStampedModel): ) # portfolio - portfolio = models.OneToOneField( + portfolio = models.ForeignKey( "registrar.Portfolio", on_delete=models.PROTECT, null=True, diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index dfb57cee0..1c4725be1 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -304,7 +304,7 @@ class DomainRequest(TimeStampedModel): ) # portfolio - portfolio = models.OneToOneField( + portfolio = models.ForeignKey( "registrar.Portfolio", on_delete=models.PROTECT, null=True, diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 14de445c2..b3131cb87 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -21,18 +21,15 @@ class Portfolio(TimeStampedModel): OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices - # creator - stores who created this model. If no creator is specified in DJA, + # Stores who created this model. If no creator is specified in DJA, # then the creator will default to the current request user""" creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", unique=False) - # notes - text field (copies what is done on domain requests) notes = models.TextField( null=True, blank=True, ) - # federal agency - FK to fed agency table (Not nullable, should default - # to the Non-federal agency value in the fed agency table) federal_agency = models.ForeignKey( "registrar.FederalAgency", on_delete=models.PROTECT, @@ -41,7 +38,6 @@ class Portfolio(TimeStampedModel): default=get_default_federal_agency, ) - # organization type - should match organization types allowed on domain info organization_type = models.CharField( max_length=255, choices=OrganizationChoices.choices, @@ -50,34 +46,29 @@ class Portfolio(TimeStampedModel): help_text="Type of organization", ) - # organization name - # NOTE: org name will be the same as federal agency, if it is federal, - # otherwise it will be the actual org name. If nothing is entered for - # org name and it is a federal organization, have this field fill with - # the federal agency text name. organization_name = models.CharField( null=True, blank=True, ) - # address_line1 address_line1 = models.CharField( null=True, blank=True, verbose_name="address line 1", ) - # address_line2 + address_line2 = models.CharField( null=True, blank=True, verbose_name="address line 2", ) - # city + city = models.CharField( null=True, blank=True, ) - # state (copied from domain_request.py -- imports enums from domain_request.py) + + # (imports enums from domain_request.py) state_territory = models.CharField( max_length=2, choices=StateTerritoryChoices.choices, @@ -85,14 +76,14 @@ class Portfolio(TimeStampedModel): blank=True, verbose_name="state / territory", ) - # zipcode + zipcode = models.CharField( max_length=10, null=True, blank=True, verbose_name="zip code", ) - # urbanization + urbanization = models.CharField( null=True, blank=True, @@ -100,7 +91,6 @@ class Portfolio(TimeStampedModel): verbose_name="urbanization", ) - # security_contact_email security_contact_email = models.EmailField( null=True, blank=True, From 9c885396138b8aa4bab3a1bdd26b7c9fd09cfb26 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 18 Jun 2024 08:17:08 -0400 Subject: [PATCH 21/25] updated export_tables to use Paginator --- .../management/commands/export_tables.py | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index 7d7521c1f..dddf2cada 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -1,5 +1,5 @@ +from django.core.paginator import Paginator import logging -import math import os import pyzipper import tablib @@ -56,8 +56,9 @@ class Command(BaseCommand): os.remove(f"tmp/{file_path}") logger.info(f"Removed {file_path}") + def export_table(self, table_name): - """Export a given table to a csv file in the tmp directory""" + """Export a given table to csv files in the tmp directory""" resourcename = f"{table_name}Resource" try: resourceclass = getattr(registrar.admin, resourcename) @@ -67,25 +68,25 @@ class Command(BaseCommand): # Determine the number of rows per file rows_per_file = 10000 - total_rows = len(dataset) - # Calculate the number of files needed - num_files = math.ceil(total_rows / rows_per_file) + # Use Paginator to handle splitting the dataset + paginator = Paginator(dataset.dict, rows_per_file) + num_files = paginator.num_pages logger.info(f"splitting {table_name} into {num_files} files") - # Split the dataset and export each chunk to a separate file - for i in range(num_files): - start_row = i * rows_per_file - end_row = start_row + rows_per_file + # Export each page to a separate file + for page_num in paginator.page_range: + page = paginator.page(page_num) # Create a new dataset for the chunk chunk = tablib.Dataset(headers=dataset.headers) - for row in dataset[start_row:end_row]: + for row_dict in page.object_list: + row = [row_dict[header] for header in dataset.headers] chunk.append(row) # Export the chunk to a new file - filename = f"tmp/{table_name}_{i + 1}.csv" + filename = f"tmp/{table_name}_{page_num}.csv" with open(filename, "w") as f: f.write(chunk.export("csv")) From 29fa4b26ae3c989c12b6e3f80e31fee2c72bfaaf Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 18 Jun 2024 08:21:18 -0400 Subject: [PATCH 22/25] formatted for linter --- src/registrar/management/commands/export_tables.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/export_tables.py b/src/registrar/management/commands/export_tables.py index dddf2cada..e934c9fad 100644 --- a/src/registrar/management/commands/export_tables.py +++ b/src/registrar/management/commands/export_tables.py @@ -56,7 +56,6 @@ class Command(BaseCommand): os.remove(f"tmp/{file_path}") logger.info(f"Removed {file_path}") - def export_table(self, table_name): """Export a given table to csv files in the tmp directory""" resourcename = f"{table_name}Resource" From 732cf2379b15ec83a7d13804587ef1bc79b56b44 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 18 Jun 2024 11:56:19 -0600 Subject: [PATCH 23/25] move agency function to be inside Federal Agency model --- ...3_portfolio_domaininformation_portfolio_and_more.py | 6 +++--- src/registrar/models/federal_agency.py | 5 +++++ src/registrar/models/portfolio.py | 10 +++++----- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py index df0945712..ac7a69074 100644 --- a/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py +++ b/src/registrar/migrations/0103_portfolio_domaininformation_portfolio_and_more.py @@ -1,9 +1,9 @@ -# Generated by Django 4.2.10 on 2024-06-18 03:19 +# Generated by Django 4.2.10 on 2024-06-18 17:55 from django.conf import settings from django.db import migrations, models import django.db.models.deletion -import registrar.models.portfolio +import registrar.models.federal_agency class Migration(migrations.Migration): @@ -136,7 +136,7 @@ class Migration(migrations.Migration): ( "federal_agency", models.ForeignKey( - default=registrar.models.portfolio.get_default_federal_agency, + default=registrar.models.federal_agency.FederalAgency.get_non_federal_agency, help_text="Associated federal agency", on_delete=django.db.models.deletion.PROTECT, to="registrar.federalagency", diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index cb09d12ac..521d5875a 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -230,3 +230,8 @@ class FederalAgency(TimeStampedModel): FederalAgency.objects.bulk_create(agencies) except Exception as e: logger.error(f"Error creating federal agencies: {e}") + + @classmethod + def get_non_federal_agency(cls): + """Returns the non-federal agency.""" + return FederalAgency.objects.filter(agency="Non-Federal Agency").first() \ No newline at end of file diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index b3131cb87..3382de98a 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -6,9 +6,9 @@ from registrar.models.federal_agency import FederalAgency from .utility.time_stamped_model import TimeStampedModel -def get_default_federal_agency(): - """returns non-federal agency""" - return FederalAgency.objects.filter(agency="Non-Federal Agency").first() +# def get_default_federal_agency(): +# """returns non-federal agency""" +# return FederalAgency.objects.filter(agency="Non-Federal Agency").first() class Portfolio(TimeStampedModel): @@ -20,7 +20,7 @@ class Portfolio(TimeStampedModel): # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices - + # Stores who created this model. If no creator is specified in DJA, # then the creator will default to the current request user""" creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", unique=False) @@ -35,7 +35,7 @@ class Portfolio(TimeStampedModel): on_delete=models.PROTECT, help_text="Associated federal agency", unique=False, - default=get_default_federal_agency, + default=FederalAgency.get_non_federal_agency, ) organization_type = models.CharField( From 46c2326933e09e6f79f21d7de806f8d1a8047f43 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 18 Jun 2024 12:05:04 -0600 Subject: [PATCH 24/25] linted --- src/registrar/admin.py | 2 +- src/registrar/models/federal_agency.py | 2 +- src/registrar/models/portfolio.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 25f2afe34..8085af631 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2563,7 +2563,7 @@ class PortfolioAdmin(ListHeaderAdmin): def save_model(self, request, obj, form, change): - if not obj.creator is None: + if obj.creator is not None: # ---- update creator ---- # Set the creator field to the current admin user obj.creator = request.user if request.user.is_authenticated else None diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 521d5875a..8db415bbd 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -234,4 +234,4 @@ class FederalAgency(TimeStampedModel): @classmethod def get_non_federal_agency(cls): """Returns the non-federal agency.""" - return FederalAgency.objects.filter(agency="Non-Federal Agency").first() \ No newline at end of file + return FederalAgency.objects.filter(agency="Non-Federal Agency").first() diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 3382de98a..a05422960 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -20,7 +20,7 @@ class Portfolio(TimeStampedModel): # use the short names in Django admin OrganizationChoices = DomainRequest.OrganizationChoices StateTerritoryChoices = DomainRequest.StateTerritoryChoices - + # Stores who created this model. If no creator is specified in DJA, # then the creator will default to the current request user""" creator = models.ForeignKey("registrar.User", on_delete=models.PROTECT, help_text="Associated user", unique=False) From 412a144c8cd4d48095c195a794b162fab0cc4014 Mon Sep 17 00:00:00 2001 From: CocoByte Date: Tue, 18 Jun 2024 16:42:00 -0600 Subject: [PATCH 25/25] removed stray comment --- src/registrar/admin.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 8085af631..8a691c7fa 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2576,7 +2576,6 @@ class PortfolioAdmin(ListHeaderAdmin): is_federal = obj.organization_type == DomainRequest.OrganizationChoices.FEDERAL if is_federal and obj.organization_name is None: obj.organization_name = obj.federal_agency.agency - # NOTE: What is meant by "federal agency text name?" super().save_model(request, obj, form, change)