From ed8535b695cbb0d281a74ac07ac1fcebd45bd86f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Mar 2024 13:06:53 -0600 Subject: [PATCH 01/76] Add signals --- src/registrar/admin.py | 3 +- src/registrar/fixtures_domain_requests.py | 5 +- .../0081_domainrequest_organization_type.py | 38 ++++++ src/registrar/models/domain.py | 2 +- src/registrar/models/domain_request.py | 45 ++++++- src/registrar/signals.py | 111 +++++++++++++++++- 6 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 src/registrar/migrations/0081_domainrequest_organization_type.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e58251743..d179d5549 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1095,8 +1095,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "Type of organization", { "fields": [ - "generic_org_type", - "is_election_board", + "organization_type", "federal_type", "federal_agency", "tribe_name", diff --git a/src/registrar/fixtures_domain_requests.py b/src/registrar/fixtures_domain_requests.py index 02efae5a9..ece1d0f7f 100644 --- a/src/registrar/fixtures_domain_requests.py +++ b/src/registrar/fixtures_domain_requests.py @@ -98,6 +98,8 @@ class DomainRequestFixture: def _set_non_foreign_key_fields(cls, da: DomainRequest, app: dict): """Helper method used by `load`.""" da.status = app["status"] if "status" in app else "started" + + # TODO for a future ticket: Allow for more than just "federal" here da.generic_org_type = app["generic_org_type"] if "generic_org_type" in app else "federal" da.federal_agency = ( app["federal_agency"] @@ -235,9 +237,6 @@ class DomainFixture(DomainRequestFixture): ).last() logger.debug(f"Approving {domain_request} for {user}") - # We don't want fixtures sending out real emails to - # fake email addresses, so we just skip that and log it instead - # All approvals require an investigator, so if there is none, # assign one. if domain_request.investigator is None: diff --git a/src/registrar/migrations/0081_domainrequest_organization_type.py b/src/registrar/migrations/0081_domainrequest_organization_type.py new file mode 100644 index 000000000..1d2185fbb --- /dev/null +++ b/src/registrar/migrations/0081_domainrequest_organization_type.py @@ -0,0 +1,38 @@ +# Generated by Django 4.2.10 on 2024-03-29 15:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0080_create_groups_v09"), + ] + + operations = [ + migrations.AddField( + model_name="domainrequest", + name="organization_type", + field=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"), + ("state_or_territory_election", "State or territory - Election"), + ("tribal_election", "Tribal - Election"), + ("county_election", "County - Election"), + ("city_election", "City - Election"), + ("special_district_election", "Special district - Election"), + ], + help_text="Type of organization - Election office", + max_length=255, + null=True, + ), + ), + ] diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 8fc697df5..b3d5b19ce 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -198,7 +198,7 @@ class Domain(TimeStampedModel, DomainHelper): is called in the validate function on the request/domain page throws- RegistryError or InvalidDomainError""" - + return True if not cls.string_could_be_domain(domain): logger.warning("Not a valid domain: %s" % str(domain)) # throw invalid domain error so that it can be caught in diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f4581de93..2b08bf1d0 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -100,8 +100,8 @@ class DomainRequest(TimeStampedModel): class OrganizationChoices(models.TextChoices): """ Primary organization choices: - For use in django admin - Keys need to match OrganizationChoicesVerbose + For use in the request experience + Keys need to match OrganizationChoicesElectionOffice and OrganizationChoicesVerbose """ FEDERAL = "federal", "Federal" @@ -113,9 +113,38 @@ class DomainRequest(TimeStampedModel): SPECIAL_DISTRICT = "special_district", "Special district" SCHOOL_DISTRICT = "school_district", "School district" + class OrganizationChoicesElectionOffice(models.TextChoices): + """ + Primary organization choices for Django admin: + Keys need to match OrganizationChoices and OrganizationChoicesVerbose. + + The enums here come in two variants: + Regular (matches the choices from OrganizationChoices) + Election (Appends " - Election" to the string) + + When adding the election variant, you must append "_election" to the end of the string. + """ + # We can't inherit OrganizationChoices due to models.TextChoices being an enum. + # We can redefine these values instead. + FEDERAL = "federal", "Federal" + INTERSTATE = "interstate", "Interstate" + STATE_OR_TERRITORY = "state_or_territory", "State or territory" + TRIBAL = "tribal", "Tribal" + COUNTY = "county", "County" + CITY = "city", "City" + SPECIAL_DISTRICT = "special_district", "Special district" + SCHOOL_DISTRICT = "school_district", "School district" + + # Election variants + STATE_OR_TERRITORY_ELECTION = "state_or_territory_election", "State or territory - Election" + TRIBAL_ELECTION = "tribal_election", "Tribal - Election" + COUNTY_ELECTION = "county_election", "County - Election" + CITY_ELECTION = "city_election", "City - Election" + SPECIAL_DISTRICT_ELECTION = "special_district_election", "Special district - Election" + class OrganizationChoicesVerbose(models.TextChoices): """ - Secondary organization choices + Tertiary organization choices For use in the domain request form and on the templates Keys need to match OrganizationChoices """ @@ -406,6 +435,14 @@ class DomainRequest(TimeStampedModel): help_text="Type of organization", ) + organization_type = models.CharField( + max_length=255, + choices=OrganizationChoicesElectionOffice.choices, + null=True, + blank=True, + help_text="Type of organization - Election office", + ) + federally_recognized_tribe = models.BooleanField( null=True, help_text="Is the tribe federally recognized", @@ -449,6 +486,7 @@ class DomainRequest(TimeStampedModel): help_text="Organization name", db_index=True, ) + address_line1 = models.CharField( null=True, blank=True, @@ -525,6 +563,7 @@ class DomainRequest(TimeStampedModel): related_name="domain_request", on_delete=models.PROTECT, ) + alternative_domains = models.ManyToManyField( "registrar.Website", blank=True, diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 4e7768ef4..e44e53ace 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -1,14 +1,121 @@ import logging -from django.db.models.signals import post_save +from django.db.models.signals import pre_save, post_save from django.dispatch import receiver -from .models import User, Contact +from .models import User, Contact, DomainRequest logger = logging.getLogger(__name__) +@receiver(pre_save, sender=DomainRequest) +def create_or_update_organization_type(sender, instance, **kwargs): + """The organization_type field on DomainRequest is consituted from the + generic_org_type and is_election_board fields. To keep the organization_type + field up to date, we need to update it before save based off of those field + values. + + If the instance is marked as an election board and the generic_org_type is not + one of the excluded types (FEDERAL, INTERSTATE, SCHOOL_DISTRICT), the + organization_type is set to a corresponding election variant. Otherwise, it directly + mirrors the generic_org_type value. + """ + if not isinstance(instance, DomainRequest): + # I don't see how this could possibly happen - but its still a good check to have. + # Lets force a fail condition rather than wait for one to happen, if this occurs. + raise ValueError("Type mismatch. The instance was not DomainRequest.") + + # == Init variables == # + # We can't grab the election variant if it is in federal, interstate, or school_district. + # The "election variant" is just the org name, with " - Election" appended to the end. + # For example, "School district - Election". + invalid_types = [ + DomainRequest.OrganizationChoices.FEDERAL, + DomainRequest.OrganizationChoices.INTERSTATE, + DomainRequest.OrganizationChoices.SCHOOL_DISTRICT, + ] + + # TODO - maybe we need a check here for .filter then .get + is_new_instance = instance.id is None + + # A new record is added with organization_type not defined. + # This happens from the regular domain request flow. + if is_new_instance: + + # == Check for invalid conditions before proceeding == # + if instance.organization_type and instance.generic_org_type: + # Since organization type is linked with generic_org_type and election board, + # we have to update one or the other, not both. + raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") + + # If no changes occurred, do nothing + if not instance.organization_type and not instance.generic_org_type: + return None + # == Program flow will halt here if there is no reason to update == # + + # == Update the linked values == # + # Find out which field needs updating + organization_type_needs_update = instance.organization_type is None + generic_org_type_needs_update = instance.generic_org_type is None + + # Update that field + if organization_type_needs_update: + _update_org_type_from_generic_org_and_election(instance, invalid_types) + elif generic_org_type_needs_update: + _update_generic_org_and_election_from_org_type(instance) + else: + + # Instance is already in the database, fetch its current state + current_instance = DomainRequest.objects.get(id=instance.id) + + # Check the new and old values + generic_org_type_changed = instance.generic_org_type != current_instance.generic_org_type + is_election_board_changed = instance.is_election_board != current_instance.is_election_board + organization_type_changed = instance.organization_type != current_instance.organization_type + + # == Check for invalid conditions before proceeding == # + if organization_type_changed and (generic_org_type_changed or is_election_board_changed): + # Since organization type is linked with generic_org_type and election board, + # we have to update one or the other, not both. + raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") + + # If no changes occured, do nothing + if not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): + return None + # == Program flow will halt here if there is no reason to update == # + + # == Update the linked values == # + # Find out which field needs updating + organization_type_needs_update = generic_org_type_changed or is_election_board_changed + generic_org_type_needs_update = organization_type_changed + + # Update that field + if organization_type_needs_update: + _update_org_type_from_generic_org_and_election(instance, invalid_types) + elif generic_org_type_needs_update: + _update_generic_org_and_election_from_org_type(instance) + +def _update_org_type_from_generic_org_and_election(instance, invalid_types): + # TODO handle if generic_org_type is None + if instance.generic_org_type not in invalid_types and instance.is_election_board: + instance.organization_type = f"{instance.generic_org_type}_election" + else: + instance.organization_type = str(instance.generic_org_type) + + +def _update_generic_org_and_election_from_org_type(instance): + """Given a value for organization_type, update the + generic_org_type and is_election_board values.""" + # TODO find a better solution than this + current_org_type = str(instance.organization_type) + if "_election" in current_org_type: + instance.generic_org_type = current_org_type.split("_election")[0] + instance.is_election_board = True + else: + instance.organization_type = str(instance.generic_org_type) + instance.is_election_board = False + @receiver(post_save, sender=User) def handle_profile(sender, instance, **kwargs): """Method for when a User is saved. From 94132bb9a27603c15a116fcd163bbbae30a80890 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Mar 2024 14:11:05 -0600 Subject: [PATCH 02/76] Update signals.py --- src/registrar/signals.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index e44e53ace..c7dc8821d 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -48,9 +48,8 @@ def create_or_update_organization_type(sender, instance, **kwargs): # Since organization type is linked with generic_org_type and election board, # we have to update one or the other, not both. raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") - - # If no changes occurred, do nothing - if not instance.organization_type and not instance.generic_org_type: + elif not instance.organization_type and not instance.generic_org_type: + # Do values to update - do nothing return None # == Program flow will halt here if there is no reason to update == # @@ -79,9 +78,8 @@ def create_or_update_organization_type(sender, instance, **kwargs): # Since organization type is linked with generic_org_type and election board, # we have to update one or the other, not both. raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") - - # If no changes occured, do nothing - if not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): + elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): + # Do values to update - do nothing return None # == Program flow will halt here if there is no reason to update == # From 58b8e4649dfb61ed24c5f193d4f1bf9538b131bc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 10:12:25 -0600 Subject: [PATCH 03/76] Refactor --- src/registrar/admin.py | 3 +- ...ninformation_organization_type_and_more.py | 83 +++++++++++++ .../0081_domainrequest_organization_type.py | 38 ------ src/registrar/models/domain_information.py | 18 ++- src/registrar/models/domain_request.py | 57 +++++++-- src/registrar/signals.py | 109 ++++++++++++------ 6 files changed, 224 insertions(+), 84 deletions(-) create mode 100644 src/registrar/migrations/0081_domaininformation_organization_type_and_more.py delete mode 100644 src/registrar/migrations/0081_domainrequest_organization_type.py diff --git a/src/registrar/admin.py b/src/registrar/admin.py index d179d5549..f2204e543 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -883,8 +883,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "Type of organization", { "fields": [ - "generic_org_type", - "is_election_board", + "organization_type", "federal_type", "federal_agency", "tribe_name", diff --git a/src/registrar/migrations/0081_domaininformation_organization_type_and_more.py b/src/registrar/migrations/0081_domaininformation_organization_type_and_more.py new file mode 100644 index 000000000..8b3818b16 --- /dev/null +++ b/src/registrar/migrations/0081_domaininformation_organization_type_and_more.py @@ -0,0 +1,83 @@ +# Generated by Django 4.2.10 on 2024-04-01 15:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0080_create_groups_v09"), + ] + + operations = [ + migrations.AddField( + model_name="domaininformation", + name="organization_type", + field=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"), + ("state_or_territory_election", "State or territory - Election"), + ("tribal_election", "Tribal - Election"), + ("county_election", "County - Election"), + ("city_election", "City - Election"), + ("special_district_election", "Special district - Election"), + ], + help_text="Type of organization - Election office", + max_length=255, + null=True, + ), + ), + migrations.AddField( + model_name="domainrequest", + name="organization_type", + field=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"), + ("state_or_territory_election", "State or territory - Election"), + ("tribal_election", "Tribal - Election"), + ("county_election", "County - Election"), + ("city_election", "City - Election"), + ("special_district_election", "Special district - Election"), + ], + help_text="Type of organization - Election office", + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="domaininformation", + name="generic_org_type", + field=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, + ), + ), + ] diff --git a/src/registrar/migrations/0081_domainrequest_organization_type.py b/src/registrar/migrations/0081_domainrequest_organization_type.py deleted file mode 100644 index 1d2185fbb..000000000 --- a/src/registrar/migrations/0081_domainrequest_organization_type.py +++ /dev/null @@ -1,38 +0,0 @@ -# Generated by Django 4.2.10 on 2024-03-29 15:58 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0080_create_groups_v09"), - ] - - operations = [ - migrations.AddField( - model_name="domainrequest", - name="organization_type", - field=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"), - ("state_or_territory_election", "State or territory - Election"), - ("tribal_election", "Tribal - Election"), - ("county_election", "County - Election"), - ("city_election", "City - Election"), - ("special_district_election", "Special district - Election"), - ], - help_text="Type of organization - Election office", - max_length=255, - null=True, - ), - ), - ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index b5755a3c9..f41e7d5c6 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -54,7 +54,23 @@ class DomainInformation(TimeStampedModel): choices=OrganizationChoices.choices, null=True, blank=True, - help_text="Type of Organization", + help_text="Type of organization", + ) + + # TODO - Ticket #1911: stub this data from DomainRequest + is_election_board = models.BooleanField( + null=True, + blank=True, + help_text="Is your organization an election office?", + ) + + # TODO - Ticket #1911: stub this data from DomainRequest + organization_type = models.CharField( + max_length=255, + choices=DomainRequest.OrgChoicesElectionOffice.choices, + null=True, + blank=True, + help_text="Type of organization - Election office", ) federally_recognized_tribe = models.BooleanField( diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 2b08bf1d0..0293fd124 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -101,7 +101,7 @@ class DomainRequest(TimeStampedModel): """ Primary organization choices: For use in the request experience - Keys need to match OrganizationChoicesElectionOffice and OrganizationChoicesVerbose + Keys need to match OrgChoicesElectionOffice and OrganizationChoicesVerbose """ FEDERAL = "federal", "Federal" @@ -113,7 +113,7 @@ class DomainRequest(TimeStampedModel): SPECIAL_DISTRICT = "special_district", "Special district" SCHOOL_DISTRICT = "school_district", "School district" - class OrganizationChoicesElectionOffice(models.TextChoices): + class OrgChoicesElectionOffice(models.TextChoices): """ Primary organization choices for Django admin: Keys need to match OrganizationChoices and OrganizationChoicesVerbose. @@ -142,6 +142,44 @@ class DomainRequest(TimeStampedModel): CITY_ELECTION = "city_election", "City - Election" SPECIAL_DISTRICT_ELECTION = "special_district_election", "Special district - Election" + @classmethod + def get_org_election_to_org_generic(cls): + """ + Creates and returns a dictionary mapping from election-specific organization + choice enums to their corresponding general organization choice enums. + + If no such mapping exists, it is simple excluded from the map. + """ + # This can be mapped automatically but its harder to read. + # For clarity reasons, we manually define this. + org_election_map = { + cls.STATE_OR_TERRITORY_ELECTION: cls.STATE_OR_TERRITORY, + cls.TRIBAL_ELECTION: cls.TRIBAL, + cls.COUNTY_ELECTION: cls.COUNTY, + cls.CITY_ELECTION: cls.CITY, + cls.SPECIAL_DISTRICT_ELECTION: cls.SPECIAL_DISTRICT, + } + return org_election_map + + @classmethod + def get_org_generic_to_org_election(cls): + """ + Creates and returns a dictionary mapping from general organization + choice enums to their corresponding election-specific organization enums. + + If no such mapping exists, it is simple excluded from the map. + """ + # This can be mapped automatically but its harder to read. + # For clarity reasons, we manually define this. + org_election_map = { + cls.STATE_OR_TERRITORY: cls.STATE_OR_TERRITORY_ELECTION, + cls.TRIBAL: cls.TRIBAL_ELECTION, + cls.COUNTY: cls.COUNTY_ELECTION, + cls.CITY: cls.CITY_ELECTION, + cls.SPECIAL_DISTRICT: cls.SPECIAL_DISTRICT_ELECTION, + } + return org_election_map + class OrganizationChoicesVerbose(models.TextChoices): """ Tertiary organization choices @@ -435,9 +473,16 @@ class DomainRequest(TimeStampedModel): help_text="Type of organization", ) + is_election_board = models.BooleanField( + null=True, + blank=True, + help_text="Is your organization an election office?", + ) + + # TODO - Ticket #1911: stub this data from DomainRequest organization_type = models.CharField( max_length=255, - choices=OrganizationChoicesElectionOffice.choices, + choices=OrgChoicesElectionOffice.choices, null=True, blank=True, help_text="Type of organization - Election office", @@ -474,12 +519,6 @@ class DomainRequest(TimeStampedModel): help_text="Federal government branch", ) - is_election_board = models.BooleanField( - null=True, - blank=True, - help_text="Is your organization an election office?", - ) - organization_name = models.CharField( null=True, blank=True, diff --git a/src/registrar/signals.py b/src/registrar/signals.py index c7dc8821d..02f13a57b 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -3,15 +3,16 @@ import logging from django.db.models.signals import pre_save, post_save from django.dispatch import receiver -from .models import User, Contact, DomainRequest +from .models import User, Contact, DomainRequest, DomainInformation logger = logging.getLogger(__name__) @receiver(pre_save, sender=DomainRequest) +@receiver(pre_save, sender=DomainInformation) def create_or_update_organization_type(sender, instance, **kwargs): - """The organization_type field on DomainRequest is consituted from the + """The organization_type field on DomainRequest and DomainInformation is consituted from the generic_org_type and is_election_board fields. To keep the organization_type field up to date, we need to update it before save based off of those field values. @@ -21,50 +22,67 @@ def create_or_update_organization_type(sender, instance, **kwargs): organization_type is set to a corresponding election variant. Otherwise, it directly mirrors the generic_org_type value. """ - if not isinstance(instance, DomainRequest): + if not isinstance(instance, DomainRequest) and not isinstance(instance, DomainInformation): # I don't see how this could possibly happen - but its still a good check to have. # Lets force a fail condition rather than wait for one to happen, if this occurs. - raise ValueError("Type mismatch. The instance was not DomainRequest.") + raise ValueError("Type mismatch. The instance was not DomainRequest or DomainInformation.") # == Init variables == # - # We can't grab the election variant if it is in federal, interstate, or school_district. - # The "election variant" is just the org name, with " - Election" appended to the end. - # For example, "School district - Election". - invalid_types = [ - DomainRequest.OrganizationChoices.FEDERAL, - DomainRequest.OrganizationChoices.INTERSTATE, - DomainRequest.OrganizationChoices.SCHOOL_DISTRICT, - ] - - # TODO - maybe we need a check here for .filter then .get is_new_instance = instance.id is None + election_org_choices = DomainRequest.OrgChoicesElectionOffice + + # For any given organization type, return the "_election" variant. + # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION + generic_org_to_org_map = election_org_choices.get_org_generic_to_org_election() + + # For any given "_election" variant, return the base org type. + # For example: STATE_OR_TERRITORY_ELECTION => STATE_OR_TERRITORY + election_org_to_generic_org_map = election_org_choices.get_org_election_to_org_generic() # A new record is added with organization_type not defined. # This happens from the regular domain request flow. if is_new_instance: # == Check for invalid conditions before proceeding == # + # Since organization type is linked with generic_org_type and election board, + # we have to update one or the other, not both. if instance.organization_type and instance.generic_org_type: - # Since organization type is linked with generic_org_type and election board, - # we have to update one or the other, not both. - raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") + organization_type = str(instance.organization_type) + generic_org_type = str(instance.generic_org_type) + + # We can only proceed if all values match (fixtures, load_from_da). + # Otherwise, we're overwriting data so lets forbid this. + if ( + "_election" in organization_type != instance.is_election_board or + organization_type != generic_org_type + ): + message = ( + "Cannot add organization_type and generic_org_type simultaneously " + "when generic_org_type, is_election_board, and organization_type values do not match." + ) + raise ValueError(message) elif not instance.organization_type and not instance.generic_org_type: - # Do values to update - do nothing + # No values to update - do nothing return None # == Program flow will halt here if there is no reason to update == # # == Update the linked values == # - # Find out which field needs updating organization_type_needs_update = instance.organization_type is None generic_org_type_needs_update = instance.generic_org_type is None - # Update that field + # If a field is none, it indicates (per prior checks) that the + # related field (generic org type <-> org type) has data and we should update according to that. if organization_type_needs_update: - _update_org_type_from_generic_org_and_election(instance, invalid_types) + _update_org_type_from_generic_org_and_election(instance) elif generic_org_type_needs_update: _update_generic_org_and_election_from_org_type(instance) + else: + # This indicates that all data already matches, + # so we should just do nothing because there is nothing to update. + pass else: - + + # == Init variables == # # Instance is already in the database, fetch its current state current_instance = DomainRequest.objects.get(id=instance.id) @@ -77,6 +95,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): if organization_type_changed and (generic_org_type_changed or is_election_board_changed): # Since organization type is linked with generic_org_type and election board, # we have to update one or the other, not both. + # This will not happen in normal flow as it is not possible otherwise. raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): # Do values to update - do nothing @@ -90,28 +109,50 @@ def create_or_update_organization_type(sender, instance, **kwargs): # Update that field if organization_type_needs_update: - _update_org_type_from_generic_org_and_election(instance, invalid_types) + _update_org_type_from_generic_org_and_election(instance) elif generic_org_type_needs_update: _update_generic_org_and_election_from_org_type(instance) -def _update_org_type_from_generic_org_and_election(instance, invalid_types): - # TODO handle if generic_org_type is None - if instance.generic_org_type not in invalid_types and instance.is_election_board: - instance.organization_type = f"{instance.generic_org_type}_election" +def _update_org_type_from_generic_org_and_election(instance): + """Given a field values for generic_org_type and is_election_board, update the + organization_type field.""" + + # We convert to a string because the enum types are different. + generic_org_type = str(instance.generic_org_type) + + # For any given organization type, return the "_election" variant. + # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION + election_org_choices = DomainRequest.OrgChoicesElectionOffice + org_map = election_org_choices.get_org_generic_to_org_election() + + # This essentially means: instance.generic_org_type not in invalid_types + if generic_org_type in org_map and instance.is_election_board: + instance.organization_type = org_map[generic_org_type] else: - instance.organization_type = str(instance.generic_org_type) + instance.organization_type = generic_org_type def _update_generic_org_and_election_from_org_type(instance): - """Given a value for organization_type, update the - generic_org_type and is_election_board values.""" - # TODO find a better solution than this + """Given the field value for organization_type, update the + generic_org_type and is_election_board field.""" + + # We convert to a string because the enum types are different + # between OrgChoicesElectionOffice and OrganizationChoices. + # But their names are the same (for the most part). current_org_type = str(instance.organization_type) - if "_election" in current_org_type: - instance.generic_org_type = current_org_type.split("_election")[0] + + # For any given organization type, return the generic variant. + # For example: STATE_OR_TERRITORY_ELECTION => STATE_OR_TERRITORY + election_org_choices = DomainRequest.OrgChoicesElectionOffice + org_map = election_org_choices.get_org_election_to_org_generic() + + # This essentially means: "_election" in current_org_type + if current_org_type in org_map: + new_org = org_map[current_org_type] + instance.generic_org_type = new_org instance.is_election_board = True else: - instance.organization_type = str(instance.generic_org_type) + instance.generic_org_type = current_org_type instance.is_election_board = False @receiver(post_save, sender=User) From 198977bb6ea92846107583cd496e6cc49f30009e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 12:41:59 -0600 Subject: [PATCH 04/76] Test cases --- src/registrar/signals.py | 88 +++++++++++------ src/registrar/tests/common.py | 9 +- src/registrar/tests/test_signals.py | 148 ++++++++++++++++++++++++++++ 3 files changed, 213 insertions(+), 32 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 02f13a57b..3a90c6a5c 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -30,7 +30,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): # == Init variables == # is_new_instance = instance.id is None election_org_choices = DomainRequest.OrgChoicesElectionOffice - + # For any given organization type, return the "_election" variant. # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION generic_org_to_org_map = election_org_choices.get_org_generic_to_org_election() @@ -48,19 +48,29 @@ def create_or_update_organization_type(sender, instance, **kwargs): # we have to update one or the other, not both. if instance.organization_type and instance.generic_org_type: organization_type = str(instance.organization_type) + # Strip "_election" if it exists + mapped_org_type = election_org_to_generic_org_map.get(organization_type) generic_org_type = str(instance.generic_org_type) + should_proceed = True # We can only proceed if all values match (fixtures, load_from_da). # Otherwise, we're overwriting data so lets forbid this. - if ( - "_election" in organization_type != instance.is_election_board or - organization_type != generic_org_type - ): + is_election_type = "_election" in organization_type + can_have_election_board = organization_type in generic_org_to_org_map + if is_election_type != instance.is_election_board and can_have_election_board: + # This means that there is a mismatch between the booleans + # (i.e. FEDERAL is not equal to is_election_board = True) + should_proceed = False + elif mapped_org_type is not None and generic_org_type != mapped_org_type: + # This means that there is as mismatch between the org types + should_proceed = False + + if not should_proceed: message = ( "Cannot add organization_type and generic_org_type simultaneously " "when generic_org_type, is_election_board, and organization_type values do not match." ) - raise ValueError(message) + raise ValueError(message) elif not instance.organization_type and not instance.generic_org_type: # No values to update - do nothing return None @@ -73,15 +83,17 @@ def create_or_update_organization_type(sender, instance, **kwargs): # If a field is none, it indicates (per prior checks) that the # related field (generic org type <-> org type) has data and we should update according to that. if organization_type_needs_update: - _update_org_type_from_generic_org_and_election(instance) + _update_org_type_from_generic_org_and_election(instance, generic_org_to_org_map) elif generic_org_type_needs_update: - _update_generic_org_and_election_from_org_type(instance) + _update_generic_org_and_election_from_org_type( + instance, election_org_to_generic_org_map, generic_org_to_org_map + ) else: # This indicates that all data already matches, # so we should just do nothing because there is nothing to update. pass else: - + # == Init variables == # # Instance is already in the database, fetch its current state current_instance = DomainRequest.objects.get(id=instance.id) @@ -109,30 +121,41 @@ def create_or_update_organization_type(sender, instance, **kwargs): # Update that field if organization_type_needs_update: - _update_org_type_from_generic_org_and_election(instance) + _update_org_type_from_generic_org_and_election(instance, generic_org_to_org_map) elif generic_org_type_needs_update: - _update_generic_org_and_election_from_org_type(instance) + _update_generic_org_and_election_from_org_type( + instance, election_org_to_generic_org_map, generic_org_to_org_map + ) -def _update_org_type_from_generic_org_and_election(instance): + +def _update_org_type_from_generic_org_and_election(instance, org_map): """Given a field values for generic_org_type and is_election_board, update the organization_type field.""" # We convert to a string because the enum types are different. generic_org_type = str(instance.generic_org_type) - # For any given organization type, return the "_election" variant. - # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION - election_org_choices = DomainRequest.OrgChoicesElectionOffice - org_map = election_org_choices.get_org_generic_to_org_election() - - # This essentially means: instance.generic_org_type not in invalid_types - if generic_org_type in org_map and instance.is_election_board: - instance.organization_type = org_map[generic_org_type] - else: + # If the election board is none, then it tells us that it is an invalid field. + # Such as federal, interstate, or school_district. + if instance.is_election_board is None and generic_org_type not in org_map: instance.organization_type = generic_org_type + return instance + elif instance.is_election_board is None and generic_org_type in org_map: + # This can only happen with manual data tinkering, which causes these to be out of sync. + instance.is_election_board = False + logger.warning("create_or_update_organization_type() -> is_election_board is out of sync. Updating value.") + + if generic_org_type in org_map: + # Swap to the election type if it is an election board. Otherwise, stick to the normal one. + instance.organization_type = org_map[generic_org_type] if instance.is_election_board else generic_org_type + elif generic_org_type not in org_map: + # Election board should be reset to None if the record + # can't have one. For example, federal. + instance.organization_type = generic_org_type + instance.is_election_board = None -def _update_generic_org_and_election_from_org_type(instance): +def _update_generic_org_and_election_from_org_type(instance, election_org_map, generic_org_map): """Given the field value for organization_type, update the generic_org_type and is_election_board field.""" @@ -141,19 +164,22 @@ def _update_generic_org_and_election_from_org_type(instance): # But their names are the same (for the most part). current_org_type = str(instance.organization_type) - # For any given organization type, return the generic variant. - # For example: STATE_OR_TERRITORY_ELECTION => STATE_OR_TERRITORY - election_org_choices = DomainRequest.OrgChoicesElectionOffice - org_map = election_org_choices.get_org_election_to_org_generic() - - # This essentially means: "_election" in current_org_type - if current_org_type in org_map: - new_org = org_map[current_org_type] + # This essentially means: "_election" in current_org_type. + if current_org_type in election_org_map: + new_org = election_org_map[current_org_type] instance.generic_org_type = new_org instance.is_election_board = True else: instance.generic_org_type = current_org_type - instance.is_election_board = False + + # This basically checks if the given org type + # can even have an election board in the first place. + # For instance, federal cannot so is_election_board = None + if current_org_type in generic_org_map: + instance.is_election_board = False + else: + instance.is_election_board = None + @receiver(post_save, sender=User) def handle_profile(sender, instance, **kwargs): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 9ecc6af67..1a4120106 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -782,6 +782,9 @@ def completed_domain_request( submitter=False, name="city.gov", investigator=None, + generic_org_type="federal", + is_election_board=False, + organization_type=None, ): """A completed domain request.""" if not user: @@ -819,7 +822,8 @@ def completed_domain_request( is_staff=True, ) domain_request_kwargs = dict( - generic_org_type="federal", + generic_org_type=generic_org_type, + is_election_board=is_election_board, federal_type="executive", purpose="Purpose of the site", is_policy_acknowledged=True, @@ -840,6 +844,9 @@ def completed_domain_request( if has_anything_else: domain_request_kwargs["anything_else"] = "There is more" + if organization_type: + domain_request_kwargs["organization_type"] = organization_type + domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_signals.py b/src/registrar/tests/test_signals.py index 4e2cbc83b..a6f8adb44 100644 --- a/src/registrar/tests/test_signals.py +++ b/src/registrar/tests/test_signals.py @@ -2,6 +2,8 @@ from django.test import TestCase from django.contrib.auth import get_user_model from registrar.models import Contact +from registrar.models.domain_request import DomainRequest +from registrar.tests.common import completed_domain_request class TestUserPostSave(TestCase): @@ -99,3 +101,149 @@ class TestUserPostSave(TestCase): self.assertEqual(actual.last_name, self.last_name) self.assertEqual(actual.email, self.email) self.assertEqual(actual.phone, self.phone) + + +class TestDomainRequestSignals(TestCase): + """Tests hooked signals on the DomainRequest object""" + + def tearDown(self): + DomainRequest.objects.all().delete() + super().tearDown() + + def test_create_or_update_organization_type_new_instance(self): + """Test create_or_update_organization_type when creating a new instance""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + ) + + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + def test_create_or_update_organization_type_new_instance_federal_does_nothing(self): + """Test if create_or_update_organization_type does nothing when creating a new instance for federal""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + ) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.FEDERAL) + + def test_create_or_update_organization_type_existing_instance_updates_election_board(self): + """Test create_or_update_organization_type for an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_request.is_election_board = True + domain_request.save() + + self.assertEqual(domain_request.is_election_board, True) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + # Try reverting the election board value + domain_request.is_election_board = False + domain_request.save() + + self.assertEqual(domain_request.is_election_board, False) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + + # Try reverting setting an invalid value for election board (should revert to False) + domain_request.is_election_board = None + domain_request.save() + + self.assertEqual(domain_request.is_election_board, False) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + + def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): + """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + ) + + domain_request.generic_org_type = DomainRequest.OrganizationChoices.INTERSTATE + domain_request.save() + + # Election board should be None because interstate cannot have an election board. + self.assertEqual(domain_request.is_election_board, None) + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.INTERSTATE) + + # Try changing the org Type to something that CAN have an election board. + domain_request_tribal = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="startedTribal.gov", + generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, + is_election_board=True, + ) + self.assertEqual( + domain_request_tribal.organization_type, DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION + ) + + # Change the org type + domain_request_tribal.generic_org_type = DomainRequest.OrganizationChoices.STATE_OR_TERRITORY + domain_request_tribal.save() + + self.assertEqual(domain_request_tribal.is_election_board, True) + self.assertEqual( + domain_request_tribal.organization_type, DomainRequest.OrgChoicesElectionOffice.STATE_OR_TERRITORY_ELECTION + ) + + def test_create_or_update_organization_type_no_update(self): + """Test create_or_update_organization_type when there are no values to update.""" + + # Test for when both generic_org_type and organization_type is declared, + # and are both non-election board + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_request.save() + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + self.assertEqual(domain_request.is_election_board, False) + self.assertEqual(domain_request.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Test for when both generic_org_type and organization_type is declared, + # and are both election board + domain_request_election = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="startedElection.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + organization_type=DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION, + ) + + self.assertEqual( + domain_request_election.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION + ) + self.assertEqual(domain_request_election.is_election_board, True) + self.assertEqual(domain_request_election.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Modify an unrelated existing value for both, and ensure that everything is still consistent + domain_request.city = "Fudge" + domain_request_election.city = "Caramel" + domain_request.save() + domain_request_election.save() + + self.assertEqual(domain_request.city, "Fudge") + self.assertEqual(domain_request_election.city, "Caramel") + + # Test for non-election + self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + self.assertEqual(domain_request.is_election_board, False) + self.assertEqual(domain_request.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Test for election + self.assertEqual( + domain_request_election.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION + ) + self.assertEqual(domain_request_election.is_election_board, True) + self.assertEqual(domain_request_election.generic_org_type, DomainRequest.OrganizationChoices.CITY) From 941512c70412004ed2f95e8d5640a1469cae2b15 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 13:47:27 -0600 Subject: [PATCH 05/76] Linting --- src/registrar/models/domain.py | 1 - src/registrar/models/domain_request.py | 3 +- src/registrar/signals.py | 81 +++++++++++++------------- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b3d5b19ce..079fce3bc 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -198,7 +198,6 @@ class Domain(TimeStampedModel, DomainHelper): is called in the validate function on the request/domain page throws- RegistryError or InvalidDomainError""" - return True if not cls.string_could_be_domain(domain): logger.warning("Not a valid domain: %s" % str(domain)) # throw invalid domain error so that it can be caught in diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 0293fd124..fc2864fe4 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -124,6 +124,7 @@ class DomainRequest(TimeStampedModel): When adding the election variant, you must append "_election" to the end of the string. """ + # We can't inherit OrganizationChoices due to models.TextChoices being an enum. # We can redefine these values instead. FEDERAL = "federal", "Federal" @@ -160,7 +161,7 @@ class DomainRequest(TimeStampedModel): cls.SPECIAL_DISTRICT_ELECTION: cls.SPECIAL_DISTRICT, } return org_election_map - + @classmethod def get_org_generic_to_org_election(cls): """ diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 3a90c6a5c..22a04b39c 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -22,13 +22,8 @@ def create_or_update_organization_type(sender, instance, **kwargs): organization_type is set to a corresponding election variant. Otherwise, it directly mirrors the generic_org_type value. """ - if not isinstance(instance, DomainRequest) and not isinstance(instance, DomainInformation): - # I don't see how this could possibly happen - but its still a good check to have. - # Lets force a fail condition rather than wait for one to happen, if this occurs. - raise ValueError("Type mismatch. The instance was not DomainRequest or DomainInformation.") # == Init variables == # - is_new_instance = instance.id is None election_org_choices = DomainRequest.OrgChoicesElectionOffice # For any given organization type, return the "_election" variant. @@ -41,38 +36,13 @@ def create_or_update_organization_type(sender, instance, **kwargs): # A new record is added with organization_type not defined. # This happens from the regular domain request flow. + is_new_instance = instance.id is None + if is_new_instance: # == Check for invalid conditions before proceeding == # - # Since organization type is linked with generic_org_type and election board, - # we have to update one or the other, not both. - if instance.organization_type and instance.generic_org_type: - organization_type = str(instance.organization_type) - # Strip "_election" if it exists - mapped_org_type = election_org_to_generic_org_map.get(organization_type) - generic_org_type = str(instance.generic_org_type) - should_proceed = True - - # We can only proceed if all values match (fixtures, load_from_da). - # Otherwise, we're overwriting data so lets forbid this. - is_election_type = "_election" in organization_type - can_have_election_board = organization_type in generic_org_to_org_map - if is_election_type != instance.is_election_board and can_have_election_board: - # This means that there is a mismatch between the booleans - # (i.e. FEDERAL is not equal to is_election_board = True) - should_proceed = False - elif mapped_org_type is not None and generic_org_type != mapped_org_type: - # This means that there is as mismatch between the org types - should_proceed = False - - if not should_proceed: - message = ( - "Cannot add organization_type and generic_org_type simultaneously " - "when generic_org_type, is_election_board, and organization_type values do not match." - ) - raise ValueError(message) - elif not instance.organization_type and not instance.generic_org_type: - # No values to update - do nothing + should_proceed = _validate_new_instance(instance, election_org_to_generic_org_map, generic_org_to_org_map) + if not should_proceed: return None # == Program flow will halt here if there is no reason to update == # @@ -88,10 +58,6 @@ def create_or_update_organization_type(sender, instance, **kwargs): _update_generic_org_and_election_from_org_type( instance, election_org_to_generic_org_map, generic_org_to_org_map ) - else: - # This indicates that all data already matches, - # so we should just do nothing because there is nothing to update. - pass else: # == Init variables == # @@ -148,7 +114,7 @@ def _update_org_type_from_generic_org_and_election(instance, org_map): if generic_org_type in org_map: # Swap to the election type if it is an election board. Otherwise, stick to the normal one. instance.organization_type = org_map[generic_org_type] if instance.is_election_board else generic_org_type - elif generic_org_type not in org_map: + else: # Election board should be reset to None if the record # can't have one. For example, federal. instance.organization_type = generic_org_type @@ -181,6 +147,43 @@ def _update_generic_org_and_election_from_org_type(instance, election_org_map, g instance.is_election_board = None +def _validate_new_instance(instance, election_org_to_generic_org_map, generic_org_to_org_map): + """ + Validates whether a new instance of DomainRequest or DomainInformation can proceed with the update + based on the consistency between organization_type, generic_org_type, and is_election_board. + """ + + # We conditionally accept both of these values to exist simultaneously, as long as + # those values do not intefere with eachother. + # Because this condition can only be triggered through a dev (no user flow), + # we throw an error if an invalid state is found here. + if instance.organization_type and instance.generic_org_type: + generic_org_type = str(instance.generic_org_type) + organization_type = str(instance.organization_type) + + # Strip "_election" if it exists + mapped_org_type = election_org_to_generic_org_map.get(organization_type) + + # Do tests on the org update for election board changes. + is_election_type = "_election" in organization_type + can_have_election_board = organization_type in generic_org_to_org_map + + election_board_mismatch = is_election_type != instance.is_election_board and can_have_election_board + org_type_mismatch = mapped_org_type is not None and generic_org_type != mapped_org_type + if election_board_mismatch or org_type_mismatch: + message = ( + "Cannot add organization_type and generic_org_type simultaneously " + "when generic_org_type, is_election_board, and organization_type values do not match." + ) + raise ValueError(message) + + should_proceed = True + return should_proceed + else: + should_proceed = not instance.organization_type and not instance.generic_org_type + return should_proceed + + @receiver(post_save, sender=User) def handle_profile(sender, instance, **kwargs): """Method for when a User is saved. From 022eb9bbaf398ad46e5d45bff9beec49b416b652 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 14:05:09 -0600 Subject: [PATCH 06/76] Fix bug --- src/registrar/signals.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 22a04b39c..a6fabe873 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -151,6 +151,8 @@ def _validate_new_instance(instance, election_org_to_generic_org_map, generic_or """ Validates whether a new instance of DomainRequest or DomainInformation can proceed with the update based on the consistency between organization_type, generic_org_type, and is_election_board. + + Returns a boolean determining if execution should proceed or not. """ # We conditionally accept both of these values to exist simultaneously, as long as @@ -168,8 +170,8 @@ def _validate_new_instance(instance, election_org_to_generic_org_map, generic_or is_election_type = "_election" in organization_type can_have_election_board = organization_type in generic_org_to_org_map - election_board_mismatch = is_election_type != instance.is_election_board and can_have_election_board - org_type_mismatch = mapped_org_type is not None and generic_org_type != mapped_org_type + election_board_mismatch = (is_election_type != instance.is_election_board) and can_have_election_board + org_type_mismatch = mapped_org_type is not None and (generic_org_type != mapped_org_type) if election_board_mismatch or org_type_mismatch: message = ( "Cannot add organization_type and generic_org_type simultaneously " @@ -177,11 +179,11 @@ def _validate_new_instance(instance, election_org_to_generic_org_map, generic_or ) raise ValueError(message) - should_proceed = True - return should_proceed + return True + elif not instance.organization_type and not instance.generic_org_type: + return False else: - should_proceed = not instance.organization_type and not instance.generic_org_type - return should_proceed + return True @receiver(post_save, sender=User) From 843158b3ef2de255017618fda8930c60004b92a1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 14:08:59 -0600 Subject: [PATCH 07/76] Update src/registrar/signals.py --- src/registrar/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index a6fabe873..4e2020731 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -76,7 +76,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): # This will not happen in normal flow as it is not possible otherwise. raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): - # Do values to update - do nothing + # No values to update - do nothing return None # == Program flow will halt here if there is no reason to update == # From 83e01979022da50f4eae6bca65a3d2e9cb5ee243 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 1 Apr 2024 15:38:53 -0600 Subject: [PATCH 08/76] Add DomainInformatoin tests --- src/registrar/admin.py | 1 + src/registrar/signals.py | 10 +- src/registrar/tests/common.py | 8 +- src/registrar/tests/test_admin.py | 3 +- src/registrar/tests/test_reports.py | 4 +- src/registrar/tests/test_signals.py | 164 +++++++++++++++++++++++++++- 6 files changed, 179 insertions(+), 11 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f2204e543..7c88c34a0 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -883,6 +883,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "Type of organization", { "fields": [ + "is_election_board", "organization_type", "federal_type", "federal_agency", diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 4e2020731..5cf035eb9 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -62,7 +62,15 @@ def create_or_update_organization_type(sender, instance, **kwargs): # == Init variables == # # Instance is already in the database, fetch its current state - current_instance = DomainRequest.objects.get(id=instance.id) + if isinstance(instance, DomainRequest): + current_instance = DomainRequest.objects.get(id=instance.id) + elif isinstance(instance, DomainInformation): + current_instance = DomainInformation.objects.get(id=instance.id) + else: + # This should never occur. But it never hurts to have this check anyway. + raise ValueError( + "create_or_update_organization_type() -> instance was not DomainRequest or DomainInformation" + ) # Check the new and old values generic_org_type_changed = instance.generic_org_type != current_instance.generic_org_type diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 1a4120106..9681b8cb7 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -585,7 +585,7 @@ class MockDb(TestCase): generic_org_type="federal", federal_agency="World War I Centennial Commission", federal_type="executive", - is_election_board=True, + is_election_board=False, ) self.domain_information_2, _ = DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_2, generic_org_type="interstate", is_election_board=True @@ -595,14 +595,14 @@ class MockDb(TestCase): domain=self.domain_3, generic_org_type="federal", federal_agency="Armed Forces Retirement Home", - is_election_board=True, + is_election_board=False, ) self.domain_information_4, _ = DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_4, generic_org_type="federal", federal_agency="Armed Forces Retirement Home", - is_election_board=True, + is_election_board=False, ) self.domain_information_5, _ = DomainInformation.objects.get_or_create( creator=self.user, @@ -652,7 +652,7 @@ class MockDb(TestCase): generic_org_type="federal", federal_agency="World War I Centennial Commission", federal_type="executive", - is_election_board=True, + is_election_board=False, ) self.domain_information_12, _ = DomainInformation.objects.get_or_create( creator=self.user, diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 7c0c81db4..46b5e104a 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -1453,12 +1453,13 @@ class TestDomainRequestAdmin(MockEppLib): "creator", "investigator", "generic_org_type", + "is_election_board", + "organization_type", "federally_recognized_tribe", "state_recognized_tribe", "tribe_name", "federal_agency", "federal_type", - "is_election_board", "organization_name", "address_line1", "address_line2", diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 5bd594a15..d3eec946d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -687,12 +687,12 @@ class HelperFunctions(MockDb): } # Test with distinct managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition, True) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 2] + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] self.assertEqual(managed_domains_sliced_at_end_date, expected_content) # Test without distinct managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 4, 1, 0, 0, 0, 0, 0, 0, 2] + expected_content = [3, 4, 1, 0, 0, 0, 0, 0, 0, 0] self.assertEqual(managed_domains_sliced_at_end_date, expected_content) def test_get_sliced_requests(self): diff --git a/src/registrar/tests/test_signals.py b/src/registrar/tests/test_signals.py index a6f8adb44..e950f39fb 100644 --- a/src/registrar/tests/test_signals.py +++ b/src/registrar/tests/test_signals.py @@ -1,8 +1,6 @@ from django.test import TestCase from django.contrib.auth import get_user_model - -from registrar.models import Contact -from registrar.models.domain_request import DomainRequest +from registrar.models import Contact, DomainRequest, Domain, DomainInformation from registrar.tests.common import completed_domain_request @@ -130,6 +128,7 @@ class TestDomainRequestSignals(TestCase): is_election_board=True, ) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.FEDERAL) + self.assertEqual(domain_request.is_election_board, None) def test_create_or_update_organization_type_existing_instance_updates_election_board(self): """Test create_or_update_organization_type for an existing instance.""" @@ -247,3 +246,162 @@ class TestDomainRequestSignals(TestCase): ) self.assertEqual(domain_request_election.is_election_board, True) self.assertEqual(domain_request_election.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + +class TestDomainInformationSignals(TestCase): + """Tests hooked signals on the DomainRequest object""" + + def tearDown(self): + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + Domain.objects.all().delete() + super().tearDown() + + def test_create_or_update_organization_type_new_instance(self): + """Test create_or_update_organization_type when creating a new instance""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + ) + + domain_information = DomainInformation.create_from_da(domain_request) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + def test_create_or_update_organization_type_new_instance_federal_does_nothing(self): + """Test if create_or_update_organization_type does nothing when creating a new instance for federal""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + ) + + domain_information = DomainInformation.create_from_da(domain_request) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.FEDERAL) + self.assertEqual(domain_information.is_election_board, None) + + def test_create_or_update_organization_type_existing_instance_updates_election_board(self): + """Test create_or_update_organization_type for an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_information = DomainInformation.create_from_da(domain_request) + domain_information.is_election_board = True + domain_information.save() + + self.assertEqual(domain_information.is_election_board, True) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION) + + # Try reverting the election board value + domain_information.is_election_board = False + domain_information.save() + domain_information.refresh_from_db() + + self.assertEqual(domain_information.is_election_board, False) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + + # Try reverting setting an invalid value for election board (should revert to False) + domain_information.is_election_board = None + domain_information.save() + + self.assertEqual(domain_information.is_election_board, False) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + + def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): + """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + ) + domain_information = DomainInformation.create_from_da(domain_request) + + domain_information.generic_org_type = DomainRequest.OrganizationChoices.INTERSTATE + domain_information.save() + + # Election board should be None because interstate cannot have an election board. + self.assertEqual(domain_information.is_election_board, None) + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.INTERSTATE) + + # Try changing the org Type to something that CAN have an election board. + domain_request_tribal = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="startedTribal.gov", + generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, + is_election_board=True, + ) + domain_information_tribal = DomainInformation.create_from_da(domain_request_tribal) + self.assertEqual( + domain_information_tribal.organization_type, DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION + ) + + # Change the org type + domain_information_tribal.generic_org_type = DomainRequest.OrganizationChoices.STATE_OR_TERRITORY + domain_information_tribal.save() + + self.assertEqual(domain_information_tribal.is_election_board, True) + self.assertEqual( + domain_information_tribal.organization_type, + DomainRequest.OrgChoicesElectionOffice.STATE_OR_TERRITORY_ELECTION, + ) + + def test_create_or_update_organization_type_no_update(self): + """Test create_or_update_organization_type when there are no values to update.""" + + # Test for when both generic_org_type and organization_type is declared, + # and are both non-election board + domain_request = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="started.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=False, + ) + domain_information = DomainInformation.create_from_da(domain_request) + domain_information.save() + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + self.assertEqual(domain_information.is_election_board, False) + self.assertEqual(domain_information.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Test for when both generic_org_type and organization_type is declared, + # and are both election board + domain_request_election = completed_domain_request( + status=DomainRequest.DomainRequestStatus.STARTED, + name="startedElection.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + is_election_board=True, + organization_type=DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION, + ) + domain_information_election = DomainInformation.create_from_da(domain_request_election) + + self.assertEqual( + domain_information_election.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION + ) + self.assertEqual(domain_information_election.is_election_board, True) + self.assertEqual(domain_information_election.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Modify an unrelated existing value for both, and ensure that everything is still consistent + domain_information.city = "Fudge" + domain_information_election.city = "Caramel" + domain_information.save() + domain_information_election.save() + + self.assertEqual(domain_information.city, "Fudge") + self.assertEqual(domain_information_election.city, "Caramel") + + # Test for non-election + self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) + self.assertEqual(domain_information.is_election_board, False) + self.assertEqual(domain_information.generic_org_type, DomainRequest.OrganizationChoices.CITY) + + # Test for election + self.assertEqual( + domain_information_election.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY_ELECTION + ) + self.assertEqual(domain_information_election.is_election_board, True) + self.assertEqual(domain_information_election.generic_org_type, DomainRequest.OrganizationChoices.CITY) From 79daf4c65176060d62fe82af2c3811bb9f0148ab Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Apr 2024 09:11:05 -0600 Subject: [PATCH 09/76] Update test --- src/registrar/signals.py | 2 +- src/registrar/tests/test_reports.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 5cf035eb9..301459f93 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -93,7 +93,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): organization_type_needs_update = generic_org_type_changed or is_election_board_changed generic_org_type_needs_update = organization_type_changed - # Update that field + # Update the field if organization_type_needs_update: _update_org_type_from_generic_org_and_election(instance, generic_org_to_org_map) elif generic_org_type_needs_update: diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index d3eec946d..459ccde0f 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -562,7 +562,7 @@ class ExportDataTest(MockDb, MockEppLib): "MANAGED DOMAINS COUNTS AT END DATE\n" "Total,Federal,Interstate,State or territory,Tribal,County,City," "Special district,School district,Election office\n" - "3,2,1,0,0,0,0,0,0,2\n" + "3,2,1,0,0,0,0,0,0,0\n" "\n" "Domain name,Domain type,Domain manager email 1,Domain manager email 2,Domain manager email 3\n" "cdomain11.govFederal-Executivemeoward@rocks.com\n" From 7c8d7d293c5fda1c430fb6749190864a2d44b845 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Apr 2024 13:50:15 -0600 Subject: [PATCH 10/76] Simplify _update_org_type_from_generic_org_and_election --- src/registrar/signals.py | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 301459f93..135336b04 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -108,25 +108,18 @@ def _update_org_type_from_generic_org_and_election(instance, org_map): # We convert to a string because the enum types are different. generic_org_type = str(instance.generic_org_type) - - # If the election board is none, then it tells us that it is an invalid field. - # Such as federal, interstate, or school_district. - if instance.is_election_board is None and generic_org_type not in org_map: - instance.organization_type = generic_org_type - return instance - elif instance.is_election_board is None and generic_org_type in org_map: - # This can only happen with manual data tinkering, which causes these to be out of sync. - instance.is_election_board = False - logger.warning("create_or_update_organization_type() -> is_election_board is out of sync. Updating value.") - - if generic_org_type in org_map: - # Swap to the election type if it is an election board. Otherwise, stick to the normal one. - instance.organization_type = org_map[generic_org_type] if instance.is_election_board else generic_org_type - else: - # Election board should be reset to None if the record + if generic_org_type not in generic_org_type: + # Election board should always be reset to None if the record # can't have one. For example, federal. - instance.organization_type = generic_org_type instance.is_election_board = None + instance.organization_type = generic_org_type + else: + # This can only happen with manual data tinkering, which causes these to be out of sync. + if instance.is_election_board is None: + logger.warning("create_or_update_organization_type() -> is_election_board is out of sync. Updating value.") + instance.is_election_board = False + + instance.organization_type = org_map[generic_org_type] if instance.is_election_board else generic_org_type def _update_generic_org_and_election_from_org_type(instance, election_org_map, generic_org_map): From 20067eec699715607cd4f19b69494eb2fa2fa8b2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Apr 2024 14:19:04 -0600 Subject: [PATCH 11/76] Fix typo --- src/registrar/signals.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index 135336b04..ad287219d 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -108,7 +108,7 @@ def _update_org_type_from_generic_org_and_election(instance, org_map): # We convert to a string because the enum types are different. generic_org_type = str(instance.generic_org_type) - if generic_org_type not in generic_org_type: + if generic_org_type not in org_map: # Election board should always be reset to None if the record # can't have one. For example, federal. instance.is_election_board = None From 70bd79516abc00ef875aec3013eff9b93bbcb8ec Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Apr 2024 15:38:12 -0600 Subject: [PATCH 12/76] Use sender --- src/registrar/signals.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/registrar/signals.py b/src/registrar/signals.py index ad287219d..d106f974c 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -11,7 +11,7 @@ logger = logging.getLogger(__name__) @receiver(pre_save, sender=DomainRequest) @receiver(pre_save, sender=DomainInformation) -def create_or_update_organization_type(sender, instance, **kwargs): +def create_or_update_organization_type(sender: DomainRequest | DomainInformation, instance, **kwargs): """The organization_type field on DomainRequest and DomainInformation is consituted from the generic_org_type and is_election_board fields. To keep the organization_type field up to date, we need to update it before save based off of those field @@ -62,15 +62,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): # == Init variables == # # Instance is already in the database, fetch its current state - if isinstance(instance, DomainRequest): - current_instance = DomainRequest.objects.get(id=instance.id) - elif isinstance(instance, DomainInformation): - current_instance = DomainInformation.objects.get(id=instance.id) - else: - # This should never occur. But it never hurts to have this check anyway. - raise ValueError( - "create_or_update_organization_type() -> instance was not DomainRequest or DomainInformation" - ) + current_instance = sender.objects.get(id=instance.id) # Check the new and old values generic_org_type_changed = instance.generic_org_type != current_instance.generic_org_type From 36a86533de363e08b7f4e621e43807fdbf125424 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:18:42 -0600 Subject: [PATCH 13/76] Remove website and draftdomain --- src/registrar/models/user_group.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index e8636a462..604004b19 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -36,11 +36,6 @@ class UserGroup(Group): "model": "domain", "permissions": ["view_domain"], }, - { - "app_label": "registrar", - "model": "draftdomain", - "permissions": ["change_draftdomain"], - }, { "app_label": "registrar", "model": "user", @@ -51,11 +46,6 @@ class UserGroup(Group): "model": "domaininvitation", "permissions": ["add_domaininvitation", "view_domaininvitation"], }, - { - "app_label": "registrar", - "model": "website", - "permissions": ["change_website"], - }, { "app_label": "registrar", "model": "userdomainrole", From a66e873edfcb9f4d011c4cf8a4a6c5834872dd3a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 09:24:58 -0600 Subject: [PATCH 14/76] Fixtures --- .../migrations/0081_create_groups_v10.py | 2 +- .../migrations/0082_create_groups_v11.py | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 src/registrar/migrations/0082_create_groups_v11.py diff --git a/src/registrar/migrations/0081_create_groups_v10.py b/src/registrar/migrations/0081_create_groups_v10.py index d65b6dbd2..5d8e3dbda 100644 --- a/src/registrar/migrations/0081_create_groups_v10.py +++ b/src/registrar/migrations/0081_create_groups_v10.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0080_create_groups_v09"), + ("registrar", "0080_create_groups_v10"), ] operations = [ diff --git a/src/registrar/migrations/0082_create_groups_v11.py b/src/registrar/migrations/0082_create_groups_v11.py new file mode 100644 index 000000000..73f54fb2f --- /dev/null +++ b/src/registrar/migrations/0082_create_groups_v11.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", "0081_create_groups_v09"), + ] + + operations = [ + migrations.RunPython( + create_groups, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] From c1ed009a055a3e5a3cc048be5bc25564a0e8dd34 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 10:57:05 -0600 Subject: [PATCH 15/76] Add migrations and selective model view --- src/registrar/admin.py | 40 ++++++++++++++++++- .../migrations/0081_create_groups_v10.py | 2 +- .../migrations/0082_create_groups_v11.py | 2 +- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e0c98b7c2..7ae7c1e27 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -765,6 +765,41 @@ class WebsiteAdmin(ListHeaderAdmin): "website", ] search_help_text = "Search by website." + + def get_model_perms(self, request): + """ + Return empty perms dict thus hiding the model from admin index. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if analyst_perm and not superuser_perm: + return {} + return super().get_model_perms(request) + + def has_change_permission(self, request, obj=None): + """ + Allow analysts to access the change form directly via URL. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if analyst_perm and not superuser_perm: + return True + return super().has_change_permission(request, obj) + + def response_change(self, request, obj): + """ + Override to redirect admins back to the same page after saving. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + + # Don't redirect to the website page on save if the user is an analyst. + # Rather, just redirect back to the same change page. + if analyst_perm and not superuser_perm: + opts = obj._meta + pk_value = obj._get_pk_val() + return HttpResponseRedirect(reverse('admin:%s_%s_change' % (opts.app_label, opts.model_name), args=(pk_value,))) + return super().response_change(request, obj) class UserDomainRoleAdmin(ListHeaderAdmin): @@ -1439,7 +1474,10 @@ class DomainInformationInline(admin.StackedInline): def has_change_permission(self, request, obj=None): """Custom has_change_permission override so that we can specify that analysts can edit this through this inline, but not through the model normally""" - if request.user.has_perm("registrar.analyst_access_permission"): + + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if analyst_perm and not superuser_perm: return True return super().has_change_permission(request, obj) diff --git a/src/registrar/migrations/0081_create_groups_v10.py b/src/registrar/migrations/0081_create_groups_v10.py index 5d8e3dbda..d65b6dbd2 100644 --- a/src/registrar/migrations/0081_create_groups_v10.py +++ b/src/registrar/migrations/0081_create_groups_v10.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0080_create_groups_v10"), + ("registrar", "0080_create_groups_v09"), ] operations = [ diff --git a/src/registrar/migrations/0082_create_groups_v11.py b/src/registrar/migrations/0082_create_groups_v11.py index 73f54fb2f..8bd0102cd 100644 --- a/src/registrar/migrations/0082_create_groups_v11.py +++ b/src/registrar/migrations/0082_create_groups_v11.py @@ -25,7 +25,7 @@ def create_groups(apps, schema_editor) -> Any: class Migration(migrations.Migration): dependencies = [ - ("registrar", "0081_create_groups_v09"), + ("registrar", "0081_create_groups_v10"), ] operations = [ From 269bb0cab3803cff73361a777d559dd39b203ab6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 12:19:40 -0600 Subject: [PATCH 16/76] Unit tests --- src/registrar/admin.py | 6 ++- src/registrar/tests/test_admin.py | 78 +++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7ae7c1e27..73b3e7e2a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -765,7 +765,7 @@ class WebsiteAdmin(ListHeaderAdmin): "website", ] search_help_text = "Search by website." - + def get_model_perms(self, request): """ Return empty perms dict thus hiding the model from admin index. @@ -798,7 +798,9 @@ class WebsiteAdmin(ListHeaderAdmin): if analyst_perm and not superuser_perm: opts = obj._meta pk_value = obj._get_pk_val() - return HttpResponseRedirect(reverse('admin:%s_%s_change' % (opts.app_label, opts.model_name), args=(pk_value,))) + return HttpResponseRedirect( + reverse("admin:%s_%s_change" % (opts.app_label, opts.model_name), args=(pk_value,)) + ) return super().response_change(request, obj) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 368f30721..81bf2736b 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -701,6 +701,84 @@ class TestDomainRequestAdmin(MockEppLib): ) self.mock_client = MockSESClient() + @less_console_noise_decorator + def test_analyst_can_see_alternative_domain(self): + """Tests if an analyst can still see the alternative domain field""" + + # Create fake creator + _creator = User.objects.create( + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + ) + + # Create a fake domain request + _domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator) + + fake_website = Website.objects.create(website="thisisatest.gov") + _domain_request.alternative_domains.add(fake_website) + _domain_request.save() + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, _domain_request.requested_domain.name) + + # Test if the page has the alternative domain + self.assertContains(response, "thisisatest.gov") + + # Check that the page contains the url we expect + expected_href = reverse("admin:registrar_website_change", args=[fake_website.id]) + self.assertContains(response, expected_href) + + # Navigate to the website to ensure that we can still edit it + response = self.client.get( + "/admin/registrar/website/{}/change/".format(fake_website.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, "thisisatest.gov") + + @less_console_noise_decorator + def test_analyst_can_see_current_websites(self): + """Tests if an analyst can still see current website field""" + + # Create fake creator + _creator = User.objects.create( + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + ) + + # Create a fake domain request + _domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator) + + fake_website = Website.objects.create(website="thisisatest.gov") + _domain_request.current_websites.add(fake_website) + _domain_request.save() + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, _domain_request.requested_domain.name) + + # Test if the page has the current website + self.assertContains(response, "thisisatest.gov") + def test_domain_sortable(self): """Tests if the DomainRequest sorts by domain correctly""" with less_console_noise(): From 02257c4f7846499b51b58010e4e54265d4811ca5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 12:33:49 -0600 Subject: [PATCH 17/76] fix unit test --- src/registrar/tests/test_migrations.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/tests/test_migrations.py b/src/registrar/tests/test_migrations.py index add65105a..6d8ff7151 100644 --- a/src/registrar/tests/test_migrations.py +++ b/src/registrar/tests/test_migrations.py @@ -37,7 +37,6 @@ class TestGroups(TestCase): "add_domaininvitation", "view_domaininvitation", "change_domainrequest", - "change_draftdomain", "add_federalagency", "change_federalagency", "delete_federalagency", @@ -48,7 +47,6 @@ class TestGroups(TestCase): "add_verifiedbystaff", "change_verifiedbystaff", "delete_verifiedbystaff", - "change_website", ] # Get the codenames of actual permissions associated with the group From 2f7ca1de6541496abc79b766ee58e0f7abaeef5d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 12:40:06 -0600 Subject: [PATCH 18/76] Update src/registrar/admin.py --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 73b3e7e2a..9514c569d 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -788,7 +788,7 @@ class WebsiteAdmin(ListHeaderAdmin): def response_change(self, request, obj): """ - Override to redirect admins back to the same page after saving. + Override to redirect users back to the same page after saving. """ superuser_perm = request.user.has_perm("registrar.full_access_permission") analyst_perm = request.user.has_perm("registrar.analyst_access_permission") From 7303f8055f879f5fa546d78e6be0ce19f820c587 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 13:19:04 -0600 Subject: [PATCH 19/76] DraftDomain --- src/registrar/admin.py | 37 ++++++++++++++++++++ src/registrar/tests/test_admin.py | 57 +++++++++++++++++++++++++++++-- 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9514c569d..e4990ed60 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1909,6 +1909,43 @@ class DraftDomainAdmin(ListHeaderAdmin): # in autocomplete_fields for user ordering = ["name"] + def get_model_perms(self, request): + """ + Return empty perms dict thus hiding the model from admin index. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if analyst_perm and not superuser_perm: + return {} + return super().get_model_perms(request) + + def has_change_permission(self, request, obj=None): + """ + Allow analysts to access the change form directly via URL. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + if analyst_perm and not superuser_perm: + return True + return super().has_change_permission(request, obj) + + def response_change(self, request, obj): + """ + Override to redirect users back to the same page after saving. + """ + superuser_perm = request.user.has_perm("registrar.full_access_permission") + analyst_perm = request.user.has_perm("registrar.analyst_access_permission") + + # Don't redirect to the website page on save if the user is an analyst. + # Rather, just redirect back to the same change page. + if analyst_perm and not superuser_perm: + opts = obj._meta + pk_value = obj._get_pk_val() + return HttpResponseRedirect( + reverse("admin:%s_%s_change" % (opts.app_label, opts.model_name), args=(pk_value,)) + ) + return super().response_change(request, obj) + class VerifiedByStaffAdmin(ListHeaderAdmin): list_display = ("email", "requestor", "truncated_notes", "created_at") diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 81bf2736b..74f4ba60e 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -19,7 +19,16 @@ from registrar.admin import ( UserDomainRoleAdmin, VerifiedByStaffAdmin, ) -from registrar.models import Domain, DomainRequest, DomainInformation, User, DomainInvitation, Contact, Website +from registrar.models import ( + Domain, + DomainRequest, + DomainInformation, + User, + DomainInvitation, + Contact, + Website, + DraftDomain, +) from registrar.models.user_domain_role import UserDomainRole from registrar.models.verified_by_staff import VerifiedByStaff from .common import ( @@ -702,8 +711,8 @@ class TestDomainRequestAdmin(MockEppLib): self.mock_client = MockSESClient() @less_console_noise_decorator - def test_analyst_can_see_alternative_domain(self): - """Tests if an analyst can still see the alternative domain field""" + def test_analyst_can_see_and_edit_alternative_domain(self): + """Tests if an analyst can still see and edit the alternative domain field""" # Create fake creator _creator = User.objects.create( @@ -747,6 +756,48 @@ class TestDomainRequestAdmin(MockEppLib): self.assertEqual(response.status_code, 200) self.assertContains(response, "thisisatest.gov") + @less_console_noise_decorator + def test_analyst_can_see_and_edit_requested_domain(self): + """Tests if an analyst can still see and edit the requested domain field""" + + # Create fake creator + _creator = User.objects.create( + username="MrMeoward", + first_name="Meoward", + last_name="Jones", + ) + + # Create a fake domain request + _domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW, user=_creator) + + p = "userpass" + self.client.login(username="staffuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(_domain_request.pk), + follow=True, + ) + + # Filter to get the latest from the DB (rather than direct assignment) + requested_domain = DraftDomain.objects.filter(name=_domain_request.requested_domain.name).get() + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, requested_domain.name) + + # Check that the page contains the url we expect + expected_href = reverse("admin:registrar_draftdomain_change", args=[requested_domain.id]) + self.assertContains(response, expected_href) + + # Navigate to the website to ensure that we can still edit it + response = self.client.get( + "/admin/registrar/draftdomain/{}/change/".format(requested_domain.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, "city.gov") + @less_console_noise_decorator def test_analyst_can_see_current_websites(self): """Tests if an analyst can still see current website field""" From e4c83751bfbc7cc0d2f0fc508f225104ffba4dd3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:23:02 -0600 Subject: [PATCH 20/76] Fix bug --- src/registrar/forms/domain.py | 25 +++++++++++++++++-- src/registrar/forms/domain_request_wizard.py | 24 ++++++++++++++++++ .../templates/includes/input_with_errors.html | 2 +- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 0bfd9b667..0130bd3fd 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -15,7 +15,7 @@ from registrar.utility.errors import ( SecurityEmailError, SecurityEmailErrorCodes, ) - +from django.core.validators import RegexValidator, MaxLengthValidator from ..models import Contact, DomainInformation, Domain from .common import ( ALGORITHM_CHOICES, @@ -31,7 +31,18 @@ logger = logging.getLogger(__name__) class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" - email = forms.EmailField(label="Email") + email = forms.EmailField( + label="Email", + max_length=254, + error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, + # This validator should exist in the event that a preexisting field is of invalid length + validators=[ + MaxLengthValidator( + 254, + message="Response must be less than 254 characters.", + ) + ], + ) def clean(self): """clean form data by lowercasing email""" @@ -193,6 +204,8 @@ class ContactForm(forms.ModelForm): # take off maxlength attribute for the phone number field # which interferes with out input_with_errors template tag self.fields["phone"].widget.attrs.pop("maxlength", None) + max = self.fields["email"].widget.attrs["maxlength"] + print(f"what is the max? {max}") for field_name in self.required: self.fields[field_name].required = True @@ -291,10 +304,18 @@ class DomainSecurityEmailForm(forms.Form): security_email = forms.EmailField( label="Security email (optional)", + max_length=254, required=False, error_messages={ "invalid": str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), }, + # This validator should exist in the event that a preexisting field is of invalid length + validators=[ + MaxLengthValidator( + 254, + message="Response must be less than 254 characters.", + ) + ], ) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 1efc028f6..8ef0f6e33 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -369,7 +369,15 @@ class AuthorizingOfficialForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=254, error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, + # This validator should exist in the event that a preexisting field is of invalid length + validators=[ + MaxLengthValidator( + 254, + message="Response must be less than 254 characters.", + ) + ], ) @@ -566,7 +574,15 @@ class YourContactForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=254, error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, + # This validator should exist in the event that a preexisting field is of invalid length + validators=[ + MaxLengthValidator( + 254, + message="Response must be less than 254 characters.", + ) + ], ) phone = PhoneNumberField( label="Phone", @@ -621,10 +637,18 @@ class OtherContactsForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=254, error_messages={ "required": ("Enter an email address in the required format, like name@example.com."), "invalid": ("Enter an email address in the required format, like name@example.com."), }, + # This validator should exist in the event that a preexisting field is of invalid length + validators=[ + MaxLengthValidator( + 254, + message="Response must be less than 254 characters.", + ) + ], ) phone = PhoneNumberField( label="Phone", diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index 56bd0b111..24308741d 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -76,7 +76,7 @@ error messages, if necessary. {% endif %} -{% if widget.attrs.maxlength %} +{% if widget.attrs.maxlength and show_max_length %} Date: Wed, 3 Apr 2024 15:26:05 -0600 Subject: [PATCH 21/76] Cleanup --- src/registrar/forms/domain.py | 6 ++---- .../templates/includes/input_with_errors.html | 12 ------------ 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 0130bd3fd..1a6a8778e 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -2,7 +2,7 @@ import logging from django import forms -from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator +from django.core.validators import MinValueValidator, MaxValueValidator, RegexValidator, MaxLengthValidator from django.forms import formset_factory from registrar.models import DomainRequest from phonenumber_field.widgets import RegionalPhoneNumberWidget @@ -15,7 +15,7 @@ from registrar.utility.errors import ( SecurityEmailError, SecurityEmailErrorCodes, ) -from django.core.validators import RegexValidator, MaxLengthValidator + from ..models import Contact, DomainInformation, Domain from .common import ( ALGORITHM_CHOICES, @@ -204,8 +204,6 @@ class ContactForm(forms.ModelForm): # take off maxlength attribute for the phone number field # which interferes with out input_with_errors template tag self.fields["phone"].widget.attrs.pop("maxlength", None) - max = self.fields["email"].widget.attrs["maxlength"] - print(f"what is the max? {max}") for field_name in self.required: self.fields[field_name].required = True diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index 24308741d..b6c24ba3e 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -75,15 +75,3 @@ error messages, if necessary. {% else %} {% endif %} - -{% if widget.attrs.maxlength and show_max_length %} - - You can enter up to {{ widget.attrs.maxlength }} characters - - - -{% endif %} From d8777c5dae698d808acab0ca7596f4dd70933020 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:44:57 -0600 Subject: [PATCH 22/76] Fix javascript errors --- .../templates/includes/input_with_errors.html | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index b6c24ba3e..e189f8d85 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -75,3 +75,25 @@ error messages, if necessary. {% else %} {% endif %} + +{% if widget.attrs.maxlength %} + + + +{% endif %} From d0d312598da04b347ef45af6d240b7d304078be6 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 3 Apr 2024 14:49:38 -0700 Subject: [PATCH 23/76] Edit typo on create_groups migration --- src/registrar/migrations/0037_create_groups_v01.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/migrations/0037_create_groups_v01.py b/src/registrar/migrations/0037_create_groups_v01.py index 3540ea2f3..0c04a8b61 100644 --- a/src/registrar/migrations/0037_create_groups_v01.py +++ b/src/registrar/migrations/0037_create_groups_v01.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0036 (which populates ContentType and Permissions) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] From c25a082aa091caa325cbec2ba326daaa8e59ca77 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:18:50 -0700 Subject: [PATCH 24/76] Add instructions for user group migrations --- src/registrar/models/user_group.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 2aa2f642e..6211094ec 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -5,6 +5,16 @@ logger = logging.getLogger(__name__) class UserGroup(Group): + """ + UserGroup sets read and write permissions for superusers (who have full access) + and analysts. To update analyst permissions do the following: + 1. Make desired changes to analyst group permissions in user_group.py. + 2. Follow the steps in 0037_create_groups_v01.py to create a duplicate + migration for the updated user group permissions. + 3. To migrate locally, run docker-compose up. To migrate on a sandbox, + push the new migration onto your sandbox before migrating. + """ + class Meta: verbose_name = "User group" verbose_name_plural = "User groups" @@ -49,7 +59,7 @@ class UserGroup(Group): { "app_label": "registrar", "model": "user", - "permissions": ["analyst_access_permission", "change_user"], + "permissions": ["analyst_access_permission", "change_user", "delete_user"], }, { "app_label": "registrar", From c4cf7d5669e156fb755d2034c8e42c3af49f727b Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:27:49 -0700 Subject: [PATCH 25/76] Update dependency typos in user group migrations --- src/registrar/migrations/0038_create_groups_v02.py | 2 +- src/registrar/migrations/0042_create_groups_v03.py | 2 +- src/registrar/migrations/0044_create_groups_v04.py | 2 +- src/registrar/migrations/0053_create_groups_v05.py | 2 +- src/registrar/migrations/0065_create_groups_v06.py | 2 +- src/registrar/migrations/0067_create_groups_v07.py | 2 +- src/registrar/migrations/0075_create_groups_v08.py | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/registrar/migrations/0038_create_groups_v02.py b/src/registrar/migrations/0038_create_groups_v02.py index fc61db3c0..70d13b61a 100644 --- a/src/registrar/migrations/0038_create_groups_v02.py +++ b/src/registrar/migrations/0038_create_groups_v02.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0037 (which also updates user role permissions) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0042_create_groups_v03.py b/src/registrar/migrations/0042_create_groups_v03.py index 01b7985bf..e30841599 100644 --- a/src/registrar/migrations/0042_create_groups_v03.py +++ b/src/registrar/migrations/0042_create_groups_v03.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0041 (which changes fields in domain request and domain information) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0044_create_groups_v04.py b/src/registrar/migrations/0044_create_groups_v04.py index ecb48e335..63cad49bb 100644 --- a/src/registrar/migrations/0044_create_groups_v04.py +++ b/src/registrar/migrations/0044_create_groups_v04.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0043 (which adds an expiry date field to a domain.) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0053_create_groups_v05.py b/src/registrar/migrations/0053_create_groups_v05.py index aaf74a9db..91e8389df 100644 --- a/src/registrar/migrations/0053_create_groups_v05.py +++ b/src/registrar/migrations/0053_create_groups_v05.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0052 (which alters fields in a domain request) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0065_create_groups_v06.py b/src/registrar/migrations/0065_create_groups_v06.py index d2cb32cee..965dc06a8 100644 --- a/src/registrar/migrations/0065_create_groups_v06.py +++ b/src/registrar/migrations/0065_create_groups_v06.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0065 (which renames fields in domain application) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0067_create_groups_v07.py b/src/registrar/migrations/0067_create_groups_v07.py index 85138d4af..809738ba3 100644 --- a/src/registrar/migrations/0067_create_groups_v07.py +++ b/src/registrar/migrations/0067_create_groups_v07.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0066 (which updates users with permission as Verified by Staff) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] diff --git a/src/registrar/migrations/0075_create_groups_v08.py b/src/registrar/migrations/0075_create_groups_v08.py index b0b2ed740..a4df52d21 100644 --- a/src/registrar/migrations/0075_create_groups_v08.py +++ b/src/registrar/migrations/0075_create_groups_v08.py @@ -1,5 +1,5 @@ # This migration creates the create_full_access_group and create_cisa_analyst_group groups -# It is dependent on 0035 (which populates ContentType and Permissions) +# It is dependent on 0074 (which renames Domain Application and its fields) # If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS # in the user_group model then: # [NOT RECOMMENDED] From 8e5c1aadbf841559ea6ae17f11399139a07fea72 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Wed, 3 Apr 2024 15:29:16 -0700 Subject: [PATCH 26/76] Revert user group permission changes from testing --- src/registrar/models/user_group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py index 6211094ec..3071fba11 100644 --- a/src/registrar/models/user_group.py +++ b/src/registrar/models/user_group.py @@ -59,7 +59,7 @@ class UserGroup(Group): { "app_label": "registrar", "model": "user", - "permissions": ["analyst_access_permission", "change_user", "delete_user"], + "permissions": ["analyst_access_permission", "change_user"], }, { "app_label": "registrar", From df3e628e613ed885fb382c0bac176b1c646b2d62 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 4 Apr 2024 08:33:18 -0600 Subject: [PATCH 27/76] Remove max length --- src/registrar/forms/domain.py | 4 ---- src/registrar/forms/domain_request_wizard.py | 6 ------ 2 files changed, 10 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 1a6a8778e..0d135c58b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -33,9 +33,7 @@ class DomainAddUserForm(forms.Form): email = forms.EmailField( label="Email", - max_length=254, error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, - # This validator should exist in the event that a preexisting field is of invalid length validators=[ MaxLengthValidator( 254, @@ -302,12 +300,10 @@ class DomainSecurityEmailForm(forms.Form): security_email = forms.EmailField( label="Security email (optional)", - max_length=254, required=False, error_messages={ "invalid": str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), }, - # This validator should exist in the event that a preexisting field is of invalid length validators=[ MaxLengthValidator( 254, diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 8ef0f6e33..9ac17b145 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -369,9 +369,7 @@ class AuthorizingOfficialForm(RegistrarForm): ) email = forms.EmailField( label="Email", - max_length=254, error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, - # This validator should exist in the event that a preexisting field is of invalid length validators=[ MaxLengthValidator( 254, @@ -574,9 +572,7 @@ class YourContactForm(RegistrarForm): ) email = forms.EmailField( label="Email", - max_length=254, error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, - # This validator should exist in the event that a preexisting field is of invalid length validators=[ MaxLengthValidator( 254, @@ -637,12 +633,10 @@ class OtherContactsForm(RegistrarForm): ) email = forms.EmailField( label="Email", - max_length=254, error_messages={ "required": ("Enter an email address in the required format, like name@example.com."), "invalid": ("Enter an email address in the required format, like name@example.com."), }, - # This validator should exist in the event that a preexisting field is of invalid length validators=[ MaxLengthValidator( 254, From 1e4be56e8ebb62d6bb1fa5b363b11ef28577de5e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 4 Apr 2024 08:40:40 -0600 Subject: [PATCH 28/76] 254 -> 320 characters --- src/registrar/forms/domain.py | 8 ++++---- src/registrar/forms/domain_request_wizard.py | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 0d135c58b..12417c0d2 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -36,8 +36,8 @@ class DomainAddUserForm(forms.Form): error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( - 254, - message="Response must be less than 254 characters.", + 320, + message="Response must be less than 320 characters.", ) ], ) @@ -306,8 +306,8 @@ class DomainSecurityEmailForm(forms.Form): }, validators=[ MaxLengthValidator( - 254, - message="Response must be less than 254 characters.", + 320, + message="Response must be less than 320 characters.", ) ], ) diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 9ac17b145..1e8034fe2 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -372,8 +372,8 @@ class AuthorizingOfficialForm(RegistrarForm): error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( - 254, - message="Response must be less than 254 characters.", + 320, + message="Response must be less than 320 characters.", ) ], ) @@ -575,8 +575,8 @@ class YourContactForm(RegistrarForm): error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( - 254, - message="Response must be less than 254 characters.", + 320, + message="Response must be less than 320 characters.", ) ], ) @@ -639,8 +639,8 @@ class OtherContactsForm(RegistrarForm): }, validators=[ MaxLengthValidator( - 254, - message="Response must be less than 254 characters.", + 320, + message="Response must be less than 320 characters.", ) ], ) From 6728826c0b69e84cbe53025572d8ae5eb45f735c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 4 Apr 2024 13:37:36 -0600 Subject: [PATCH 29/76] Fix max length bug --- src/registrar/forms/domain.py | 2 ++ src/registrar/forms/domain_request_wizard.py | 3 +++ .../templates/includes/input_with_errors.html | 12 +----------- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 12417c0d2..faf21e12e 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -33,6 +33,7 @@ class DomainAddUserForm(forms.Form): email = forms.EmailField( label="Email", + max_length=None, error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( @@ -300,6 +301,7 @@ class DomainSecurityEmailForm(forms.Form): security_email = forms.EmailField( label="Security email (optional)", + max_length=None, required=False, error_messages={ "invalid": str(SecurityEmailError(code=SecurityEmailErrorCodes.BAD_DATA)), diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 1e8034fe2..c3ac3b4c2 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -369,6 +369,7 @@ class AuthorizingOfficialForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=None, error_messages={"invalid": ("Enter an email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( @@ -572,6 +573,7 @@ class YourContactForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=None, error_messages={"invalid": ("Enter your email address in the required format, like name@example.com.")}, validators=[ MaxLengthValidator( @@ -633,6 +635,7 @@ class OtherContactsForm(RegistrarForm): ) email = forms.EmailField( label="Email", + max_length=None, error_messages={ "required": ("Enter an email address in the required format, like name@example.com."), "invalid": ("Enter an email address in the required format, like name@example.com."), diff --git a/src/registrar/templates/includes/input_with_errors.html b/src/registrar/templates/includes/input_with_errors.html index e189f8d85..56bd0b111 100644 --- a/src/registrar/templates/includes/input_with_errors.html +++ b/src/registrar/templates/includes/input_with_errors.html @@ -79,17 +79,7 @@ error messages, if necessary. {% if widget.attrs.maxlength %}