PR suggestions (part 1)

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.

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",
+        ),
+    ]

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,

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
 
     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
 
     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,7 +1162,6 @@ class DomainRequest(TimeStampedModel):
         ]
         if not self.sub_organization and all(required_fields):
             return True
-        else:
         return False
 
     # ## Form unlocking steps ## #
@@ -1170,10 +1169,12 @@ class DomainRequest(TimeStampedModel):
     # 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
 
     # ## Form policies ## #

src/registrar/templates/includes/header_extended.html

@@ -93,7 +93,7 @@
             </li>
             {% endif %}
 
-            {% if has_organization_members_flag %}
+            {% if has_organization_members_flag and not hide_members %}
                 <li class="usa-nav__primary-item">
                     <a href="{% url 'members' %}" class="usa-nav-link {% if path|is_members_subpage %} usa-current{% endif %}">
                         Members

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 %}
-            <p class="margin-top-0 margin-bottom-0">{{domain_request.suborganization_city}}, {{domain_request.suborganization_state_territory}}</p>
+            <p class="margin-y-0">{{domain_request.suborganization_city}}, {{domain_request.suborganization_state_territory}}</p>
         {% 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 %}

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)