From 1d345987ca9b85e17c0d63298a766ea23ae936b1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 1 Nov 2024 08:49:32 -0600 Subject: [PATCH] PR suggestions (part 1) --- src/registrar/forms/domain_request_wizard.py | 19 +++++----- ...tion_requested_suborganization_and_more.py | 25 +++++++++++++ src/registrar/models/domain_information.py | 18 ---------- src/registrar/models/domain_request.py | 35 ++++++++++--------- .../templates/includes/header_extended.html | 2 +- .../portfolio_request_review_steps.html | 2 +- src/registrar/views/domain_request.py | 10 +++++- 7 files changed, 62 insertions(+), 49 deletions(-) create mode 100644 src/registrar/migrations/0137_remove_domaininformation_requested_suborganization_and_more.py diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py index 16e5b66ea..f74622b51 100644 --- a/src/registrar/forms/domain_request_wizard.py +++ b/src/registrar/forms/domain_request_wizard.py @@ -35,7 +35,7 @@ class RequestingEntityForm(RegistrarForm): # If this selection is made on the form (tracked by js), then it will toggle the form value of this. # In other words, this essentially tracks if the suborganization field == "Other". # "Other" is just an imaginary value that is otherwise invalid. - # Note the logic in `def clean` and line 2744 in get-gov.js + # Note the logic in `def clean` and `handleRequestingEntityFieldset` in get-gov.js is_requesting_new_suborganization = forms.BooleanField(required=False, widget=forms.HiddenInput()) sub_organization = forms.ModelChoiceField( @@ -43,24 +43,22 @@ class RequestingEntityForm(RegistrarForm): required=False, queryset=Suborganization.objects.none(), empty_label="--Select--", + error_messages={ + "required": ("Requesting entity is required.") + }, ) requested_suborganization = forms.CharField( label="Requested suborganization", required=False, - error_messages={"required": "Enter the name of your organization."}, ) suborganization_city = forms.CharField( label="City", required=False, - error_messages={"required": "Enter the city where your organization is located."}, ) suborganization_state_territory = forms.ChoiceField( label="State, territory, or military post", required=False, choices=[("", "--Select--")] + DomainRequest.StateTerritoryChoices.choices, - error_messages={ - "required": ("Select the state, territory, or military post where your organization is located.") - }, ) def __init__(self, *args, **kwargs): @@ -147,17 +145,16 @@ class RequestingEntityYesNoForm(BaseYesNoForm): if self.domain_request.portfolio: self.form_choices = ( (False, self.domain_request.portfolio), - (True, "A suborganization. (choose from list)"), + (True, "A suborganization (choose from list)"), ) self.fields[self.field_name] = self.get_typed_choice_field() @property def form_is_checked(self): """ - Determines if the requesting entity is a suborganization, or a portfolio. - For suborganizations, users have the ability to request a new one if the - desired suborg doesn't exist. We expose additional fields that denote this, - like `requested_suborganization`. So we also check on those. + Determines the initial checked state of the form. + Returns True (checked) if the requesting entity is a suborganization, + and False if it is a portfolio. Returns None if neither condition is met. """ # True means that the requesting entity is a suborganization, # whereas False means that the requesting entity is a portfolio. diff --git a/src/registrar/migrations/0137_remove_domaininformation_requested_suborganization_and_more.py b/src/registrar/migrations/0137_remove_domaininformation_requested_suborganization_and_more.py new file mode 100644 index 000000000..dfa716ef4 --- /dev/null +++ b/src/registrar/migrations/0137_remove_domaininformation_requested_suborganization_and_more.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.10 on 2024-11-01 14:25 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0136_domaininformation_requested_suborganization_and_more"), + ] + + operations = [ + migrations.RemoveField( + model_name="domaininformation", + name="requested_suborganization", + ), + migrations.RemoveField( + model_name="domaininformation", + name="suborganization_city", + ), + migrations.RemoveField( + model_name="domaininformation", + name="suborganization_state_territory", + ), + ] diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 7dc257b22..7dadf26ac 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -75,24 +75,6 @@ class DomainInformation(TimeStampedModel): verbose_name="Suborganization", ) - requested_suborganization = models.CharField( - null=True, - blank=True, - ) - - suborganization_city = models.CharField( - null=True, - blank=True, - ) - - suborganization_state_territory = models.CharField( - max_length=2, - choices=StateTerritoryChoices.choices, - null=True, - blank=True, - verbose_name="state, territory, or military post", - ) - domain_request = models.OneToOneField( "registrar.DomainRequest", on_delete=models.PROTECT, diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index c3fc5335d..2a0c65505 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -1127,30 +1127,30 @@ class DomainRequest(TimeStampedModel): def requesting_entity_is_portfolio(self) -> bool: """Determines if this record is requesting that a portfolio be their organization. - Used for the RequestingEntity page.""" + Used for the RequestingEntity page. + Returns True if the portfolio exists and if organization_name matches portfolio.organization_name. + """ if self.portfolio and self.organization_name == self.portfolio.organization_name: return True - else: - return False + return False def requesting_entity_is_suborganization(self) -> bool: """Used to determine if this domain request is also requesting that it be tied to a suborganization. - Checks if this record has a suborganization or not by checking if a suborganization exists, - and if it doesn't, determining if properties like requested_suborganization exist. - Used for the RequestingEntity page. + Returns True if portfolio exists and either sub_organization exists, + or if is_requesting_new_suborganization() is true. + Returns False otherwise. """ if self.portfolio and (self.sub_organization or self.is_requesting_new_suborganization()): return True - else: - return False + return False def is_requesting_new_suborganization(self) -> bool: """Used on the requesting entity form to determine if a user is trying to request - a new suborganization using the domain request form. + a new suborganization using the domain request form, rather than one that already exists. - This only occurs when no suborganization is selected, but they've filled out - the requested_suborganization, suborganization_city, and suborganization_state_territory fields. - Used for the RequestingEntity page. + Returns True if a sub_organization does not exist and if requested_suborganization, + suborganization_city, and suborganization_state_territory all exist. + Returns False otherwise. """ # If a suborganization already exists, it can't possibly be a new one. @@ -1162,19 +1162,20 @@ class DomainRequest(TimeStampedModel): ] if not self.sub_organization and all(required_fields): return True - else: - return False + return False # ## Form unlocking steps ## # # # These methods control the conditions in which we should unlock certain domain wizard steps. def unlock_requesting_entity(self) -> bool: - """Unlocks the requesting entity step. Used for the RequestingEntity page.""" + """Unlocks the requesting entity step. Used for the RequestingEntity page. + Returns true if requesting_entity_is_suborganization() and requesting_entity_is_portfolio(). + Returns False otherwise. + """ if self.requesting_entity_is_suborganization() or self.requesting_entity_is_portfolio(): return True - else: - return False + return False # ## Form policies ## # # diff --git a/src/registrar/templates/includes/header_extended.html b/src/registrar/templates/includes/header_extended.html index 7c0f55dc3..a954eb30f 100644 --- a/src/registrar/templates/includes/header_extended.html +++ b/src/registrar/templates/includes/header_extended.html @@ -93,7 +93,7 @@ {% endif %} - {% if has_organization_members_flag %} + {% if has_organization_members_flag and not hide_members %}
  • Members diff --git a/src/registrar/templates/includes/portfolio_request_review_steps.html b/src/registrar/templates/includes/portfolio_request_review_steps.html index ed727fee8..175a9bdc1 100644 --- a/src/registrar/templates/includes/portfolio_request_review_steps.html +++ b/src/registrar/templates/includes/portfolio_request_review_steps.html @@ -14,7 +14,7 @@ {% comment %} We don't have city or state_territory for suborganizations yet, so no data should display {% endcomment %} {% elif domain_request.requesting_entity_is_suborganization %} {% include "includes/summary_item.html" with value=domain_request.requested_suborganization edit_link=domain_request_url %} -

    {{domain_request.suborganization_city}}, {{domain_request.suborganization_state_territory}}

    +

    {{domain_request.suborganization_city}}, {{domain_request.suborganization_state_territory}}

    {% elif domain_request.requesting_entity_is_portfolio %} {% include "includes/summary_item.html" with value=domain_request.portfolio.organization_name edit_link=domain_request_url %} {% if domain_request.portfolio.city and domain_request.portfolio.state_territory %} diff --git a/src/registrar/views/domain_request.py b/src/registrar/views/domain_request.py index 41e9a07b8..b1c5c9c89 100644 --- a/src/registrar/views/domain_request.py +++ b/src/registrar/views/domain_request.py @@ -319,7 +319,15 @@ class DomainRequestWizard(DomainRequestWizardPermissionView, TemplateView): # Clear context so the prop getter won't create a request here. # Creating a request will be handled in the post method for the # intro page. - return render(request, "domain_request_intro.html", {"hide_requests": True, "hide_domains": True}) + return render( + request, + "domain_request_intro.html", + { + "hide_requests": True, + "hide_domains": True, + "hide_members": True, + }, + ) else: return self.goto(self.steps.first)