Simplify logic and use better names

src/registrar/assets/js/get-gov.js

@@ -2742,48 +2742,44 @@ document.addEventListener('DOMContentLoaded', function() {
  * Within that, the dropdown also toggles some additional form elements.
 */
 (function handleRequestingEntityFieldset() { 
-  // Check if the requesting-entity-fieldset exists. 
+  // Sadly, these ugly ids are the auto generated with this prefix
+  const formPrefix = "portfolio_requesting_entity"
+
   // This determines if we are on the requesting entity page or not.
-  const fieldset = document.getElementById("requesting-entity-fieldset");
-  if (!fieldset) return;
+  const isSubOrgFieldset = document.getElementById(`id_${formPrefix}-requesting_entity_is_suborganization__fieldset`);
+  if (!isSubOrgFieldset) return;
 
   // Get the is_suborganization radio buttons
-  // Sadly, these ugly ids are the auto generated
-  const formPrefix = "portfolio_requesting_entity"
-  const isSuborgRadios = document.querySelectorAll(`input[name="${formPrefix}-is_suborganization"]`);
-  const subOrgSelect = document.querySelector(`#id_${formPrefix}-sub_organization`);
+  const isSuborgRadios = isSubOrgFieldset.querySelectorAll(`input[name="${formPrefix}-requesting_entity_is_suborganization"]`);
+  const subOrgSelect = document.getElementById(`id_${formPrefix}-sub_organization`);
 
   // The suborganization section is its own div
   // Within the suborganization section, we also have a div that contains orgname, city, and stateterritory.
-  const suborganizationFieldset = document.querySelector("#requesting-entity-fieldset__suborganization");
-  const suborganizationDetailsFieldset = document.querySelector("#requesting-entity-fieldset__suborganization__details");
+  const suborganizationContainer = document.getElementById("suborganization-container");
+  const suborganizationDetailsContainer = document.getElementById("suborganization-container__details");
 
-  // This variable determines if the user is trying to request a new suborganization or not
-  var isCustomSuborganization = document.querySelector("#id_portfolio_requesting_entity-is_custom_suborganization")
+  // This variable determines if the user is trying to *create* a new suborganization or not.
+  var isRequestingSuborganization = document.getElementById(`id_${formPrefix}-is_requesting_new_suborganization`)
 
   // Don't do anything if we are missing crucial page elements
-  if (!isSuborgRadios || !subOrgSelect || !suborganizationFieldset || !suborganizationDetailsFieldset) return;
+  if (!isSuborgRadios || !subOrgSelect || !suborganizationContainer || !suborganizationDetailsContainer) return;
 
   // Function to toggle suborganization based on is_suborganization selection
   function toggleSuborganization(radio) {
     if (radio && radio.checked && radio.value === "True") {
-      showElement(suborganizationFieldset);
-      toggleSuborganizationDetails();
-    } else {
-      hideElement(suborganizationFieldset);
-      hideElement(suborganizationDetailsFieldset);
-    }
-  };
+      showElement(suborganizationContainer);
 
-  // Function to toggle organization details based on sub_organization selection
-  function toggleSuborganizationDetails () {
-    // We should hide the org name fields when we select the special other value
+      // Handle custom suborganizations
       if (subOrgSelect.value === "other") {
-      showElement(suborganizationDetailsFieldset);
-      isCustomSuborganization.value = "True";
+        showElement(suborganizationDetailsContainer);
+        isRequestingSuborganization.value = "True";
       } else {
-      hideElement(suborganizationDetailsFieldset);
-      isCustomSuborganization.value = "False";
+        hideElement(suborganizationDetailsContainer);
+        isRequestingSuborganization.value = "False";
+      }
+    } else {
+      hideElement(suborganizationContainer);
+      hideElement(suborganizationDetailsContainer);
     }
   };
 
@@ -2795,7 +2791,8 @@ document.addEventListener('DOMContentLoaded', function() {
     subOrgSelect.add(fakeOption);
   }
 
-  if (isCustomSuborganization.value === "True") {
+  console.log(isRequestingSuborganization.value)
+  if (isRequestingSuborganization.value === "True") {
     subOrgSelect.value = "other"
   }
 
@@ -2813,6 +2810,13 @@ document.addEventListener('DOMContentLoaded', function() {
 
   // Add event listener to the suborg dropdown to show/hide the suborg details section
   subOrgSelect.addEventListener("change", () => {
-    toggleSuborganizationDetails();
+    // Handle the custom suborganization field
+    if (subOrgSelect.value === "other") {
+      showElement(suborganizationDetailsContainer);
+      isRequestingSuborganization.value = "True";
+    } else {
+      hideElement(suborganizationDetailsContainer);
+      isRequestingSuborganization.value = "False";
+    }
   });
 })();

src/registrar/forms/domain_request_wizard.py

@@ -27,6 +27,9 @@ class RequestingEntityForm(RegistrarForm):
     All of these fields are not required by default, but as we use javascript to conditionally show
     and hide some of these, they then become required in certain circumstances."""
 
+    # Add a hidden field to store if the user is requesting a new suborganization
+    is_requesting_new_suborganization = forms.BooleanField(required=False, widget=forms.HiddenInput())
+
     sub_organization = forms.ModelChoiceField(
         label="Suborganization name",
         # not required because this field won't be filled out unless
@@ -57,17 +60,6 @@ class RequestingEntityForm(RegistrarForm):
             "required": ("Select the state, territory, or military post where your organization is located.")
         },
     )
-    is_suborganization = forms.NullBooleanField(
-        widget=forms.RadioSelect(
-            choices=[
-                (True, "Yes"),
-                (False, "No"),
-            ],
-        )
-    )
-
-    # Add a hidden field to store that we are adding a custom suborg
-    is_custom_suborganization = forms.BooleanField(required=False, widget=forms.HiddenInput())
 
     def __init__(self, *args, **kwargs):
         """Override of init to add the suborganization queryset"""
@@ -81,22 +73,24 @@ class RequestingEntityForm(RegistrarForm):
     def clean_sub_organization(self):
         """On suborganization clean, set the suborganization value to None if the user is requesting
         a custom suborganization (as it doesn't exist yet)"""
-        sub_organization = self.cleaned_data.get("sub_organization")
-        is_custom = self.cleaned_data.get("is_custom_suborganization")
-        if is_custom:
-            # If it's a custom suborganization, return None (equivalent to selecting nothing)
+
+        # If it's a new suborganization, return None (equivalent to selecting nothing)
+        if self.cleaned_data.get("is_requesting_new_suborganization"):
             return None
-        return sub_organization
+
+        # Otherwise just return the suborg as normal
+        return self.cleaned_data.get("sub_organization")
 
     def full_clean(self):
         """Validation logic to remove the custom suborganization value before clean is triggered.
         Without this override, the form will throw an 'invalid option' error."""
         # Remove the custom other field before cleaning
         data = self.data.copy() if self.data else None
+
+        # Remove the 'other' value from suborganization if it exists.
+        # This is a special value that tracks if the user is requesting a new suborg.
         suborganization = self.data.get("portfolio_requesting_entity-sub_organization")
-        if suborganization:
-            if "other" in data["portfolio_requesting_entity-sub_organization"]:
-                # Remove the 'other' value
+        if suborganization and "other" in suborganization:
             data["portfolio_requesting_entity-sub_organization"] = ""
 
         # Set the modified data back to the form
@@ -110,11 +104,16 @@ class RequestingEntityForm(RegistrarForm):
         Given that these fields often rely on eachother, we need to do this in the parent function."""
         cleaned_data = super().clean()
 
+        # Do some custom error validation if the requesting entity is a suborg.
+        # Otherwise, just validate as normal.
         suborganization = self.cleaned_data.get("sub_organization")
-        is_suborganization = self.cleaned_data.get("is_suborganization")
-        is_custom_suborganization = self.cleaned_data.get("is_custom_suborganization")
-        if is_suborganization:
-            if is_custom_suborganization:
+        is_requesting_new_suborganization = self.cleaned_data.get("is_requesting_new_suborganization")
+
+        # Get the value of the yes/no checkbox from RequestingEntityYesNoForm.
+        # Since self.data stores this as a string, we need to convert "True" => True.
+        requesting_entity_is_suborganization = self.data.get("portfolio_requesting_entity-requesting_entity_is_suborganization")
+        if requesting_entity_is_suborganization == "True":
+            if is_requesting_new_suborganization:
                 # Validate custom suborganization fields
                 if not cleaned_data.get("requested_suborganization"):
                     self.add_error("requested_suborganization", "Enter details for your organization name.")
@@ -125,7 +124,6 @@ class RequestingEntityForm(RegistrarForm):
             elif not suborganization:
                 self.add_error("sub_organization", "Select a suborganization.")
 
-        cleaned_data = super().clean()
         return cleaned_data
 
 
@@ -134,7 +132,7 @@ class RequestingEntityYesNoForm(BaseYesNoForm):
 
     # This first option will change dynamically
     form_choices = ((False, "Current Organization"), (True, "A suborganization. (choose from list)"))
-    field_name = "is_suborganization"
+    field_name = "requesting_entity_is_suborganization"
 
     def __init__(self, *args, **kwargs):
         """Extend the initialization of the form from RegistrarForm __init__"""
@@ -152,12 +150,9 @@ class RequestingEntityYesNoForm(BaseYesNoForm):
         Determines the initial checked state of the form based on the domain_request's attributes.
         """
 
-        if self.domain_request.is_suborganization():
+        if self.domain_request.requesting_entity_is_suborganization():
             return True
-        elif (
-            self.domain_request.portfolio
-            and self.domain_request.organization_name == self.domain_request.portfolio.organization_name
-        ):
+        elif self.domain_request.requesting_entity_is_portfolio():
             return False
         else:
             return None

src/registrar/models/domain_request.py

@@ -1125,14 +1125,47 @@ class DomainRequest(TimeStampedModel):
 
         self.creator.restrict_user()
 
-    # Form unlocking steps
+    def requesting_entity_is_portfolio(self) -> bool:
+        """Determines if this record is requesting that a portfolio be their organization."""
+        return self.portfolio and self.organization_name == self.portfolio.organization_name
+
+    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.
+        """
+
+        if self.portfolio:
+            if self.sub_organization:
+                return True
+            if self.is_requesting_new_suborganization():
+                return True
+        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.
+
+        This only occurs when no suborganization is selected, but they've filled out
+        the requested_suborganization, suborganization_city, and suborganization_state_territory fields.
+        """
+        # If a suborganization already exists, it can't possibly be a new one
+        if self.sub_organization:
+            return False
+        return bool(self.requested_suborganization and self.suborganization_city and self.suborganization_state_territory)
+
+    # ## 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"""
-        if self.portfolio and self.organization_name == self.portfolio.organization_name:
+        if self.requesting_entity_is_suborganization():
+            return True
+        elif self.requesting_entity_is_portfolio():
             return True
         else:
-            return self.is_suborganization()
+            return False
 
     # ## Form policies ## #
     #
@@ -1203,39 +1236,6 @@ class DomainRequest(TimeStampedModel):
             return True
         return False
 
-    def is_suborganization(self) -> bool:
-        """Determines if this record is a suborganization or not by checking if a suborganization exists,
-        and if it doesn't, determining if properties like requested_suborganization exist."""
-        if self.portfolio:
-            if self.sub_organization:
-                return True
-
-            if self.has_information_required_to_make_suborganization():
-                return True
-
-        return False
-
-    def is_custom_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.
-
-        This only occurs when no suborganization is selected, but they've filled out
-        the requested_suborganization, suborganization_city, and suborganization_state_territory fields.
-        """
-        if self.is_suborganization():
-            return not self.sub_organization and self.has_information_required_to_make_suborganization()
-        else:
-            return False
-
-    def has_information_required_to_make_suborganization(self) -> bool:
-        """Checks if we have all the information we need to create a new suborganization object.
-        Checks for a the existence of requested_suborganization, suborganization_city, suborganization_state_territory
-        """
-        if self.requested_suborganization and self.suborganization_city and self.suborganization_state_territory:
-            return True
-        else:
-            return False
-
     def to_dict(self):
         """This is to process to_dict for Domain Information, making it friendly
         to "copy" it

src/registrar/templates/domain_request_requesting_entity.html

@@ -7,7 +7,7 @@
 {% endblock %}
 
 {% block form_fields %}
-    <fieldset id="requesting-entity-fieldset" class="usa-fieldset">
+    <fieldset class="usa-fieldset">
         <legend>
         <h2>Who will use the domain you’re requesting?</h2>
         </legend>
@@ -19,7 +19,7 @@
         {# forms.0 is a small yes/no form that toggles the visibility of "requesting entity" formset #}
         {% with add_class="usa-radio__input--tile" add_legend_class="usa-sr-only" %}
             {% with attr_required=True %}
-                {% input_with_errors forms.0.is_suborganization %}
+                {% input_with_errors forms.0.requesting_entity_is_suborganization %}
             {% endwith %}
         {% endwith %}
 
@@ -27,10 +27,10 @@
         was selected or not. This allows for persistence across page reloads without using session variables.
         {% endcomment %}
         {% with add_group_class="display-none" %}
-            {% input_with_errors forms.1.is_custom_suborganization %}
+            {% input_with_errors forms.1.is_requesting_new_suborganization %}
         {% endwith %}
 
-        <div id="requesting-entity-fieldset__suborganization" class="margin-top-4">
+        <div id="suborganization-container" class="margin-top-4">
             <h2>Add suborganization information</h2>
             <p>
                 This information will be published in <a class="usa-link usa-link--always-blue" href="{% public_site_url 'about/data' %}">.gov’s public data</a>. If you don’t see your suborganization in the list, 
@@ -43,7 +43,7 @@
             {% comment %} This will be toggled if a special value, "other", is selected.
             Otherwise this field is invisible. 
             {% endcomment %}
-            <div id="requesting-entity-fieldset__suborganization__details">
+            <div id="suborganization-container__details">
                 {% with attr_required=True %}
                     {% input_with_errors forms.1.requested_suborganization %}
                 {% endwith %}

src/registrar/templates/domain_request_status.html

@@ -6,9 +6,5 @@
 
 
 {% block content %}
-  {% if not portfolio %}
   {% include "includes/request_status_manage.html" %}
-  {% else %}
-    {% include "includes/portfolio_status_manage.html" %}
-  {% endif %}
 {% endblock %}

src/registrar/templates/includes/portfolio_request_review_steps.html

@@ -8,28 +8,24 @@
     {% endif %}
 
     {% if step == Step.REQUESTING_ENTITY %}
-        {% if domain_request.portfolio and domain_request.organization_name == domain_request.portfolio.organization_name %}
-            {% with title=form_titles|get_item:step value=domain_request.portfolio.organization_name %}
-                {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url %}
-            {% endwith %}
+        {% with title=form_titles|get_item:step %}
+        {% if domain_request.sub_organization %}
+            {% include "includes/summary_item.html" with value=domain_request.sub_organization edit_link=domain_request_url %}
+            {% 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>
+        {% 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 %}
                 <p class="margin-top-0 margin-bottom-0">{{domain_request.portfolio.city}}, {{domain_request.portfolio.state_territory}}</p>
             {% endif %}
-        {% elif domain_request.sub_organization %}
-            {% with title=form_titles|get_item:step value=domain_request.sub_organization %}
-                {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url %}
-            {% endwith %}
-            {% comment %} We don't have city or state_territory for suborganizations yet, so no data should display {% endcomment %}
-        {% elif domain_request.requested_suborganization and domain_request.suborganization_city and domain_request.suborganization_state_territory %}
-            {% with title=form_titles|get_item:step value=domain_request.requested_suborganization %}
-                {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url %}
-            {% endwith %}
-            <p class="margin-top-0 margin-bottom-0">{{domain_request.suborganization_city}}, {{domain_request.suborganization_state_territory}}</p>
         {% else %}
-            {% with title=form_titles|get_item:step value="<span class='text-bold text-secondary-dark'>Incomplete</span>"|safe %}
-                {% include "includes/summary_item.html" with title=title value=value heading_level=heading_level editable=is_editable edit_link=domain_request_url %}
+            {% with value="<span class='text-bold text-secondary-dark'>Incomplete</span>"|safe %}
+                {% include "includes/summary_item.html" with edit_link=domain_request_url %}
             {% endwith %}
         {% endif %}
+        {% endwith %}
     {% endif %}
 
     {% if step == Step.CURRENT_SITES %}

src/registrar/templates/includes/portfolio_status_manage.html

@@ -1,7 +0,0 @@
-{% extends "includes/request_status_manage.html" %}
-{% load custom_filters %}
-{% load static url_helpers %}
-
-{% block request_summary %}
-    {% include "includes/portfolio_request_review_steps.html" with is_editable=False domain_request=DomainRequest %}
-{% endblock request_summary %}

src/registrar/templates/includes/request_status_manage.html

@@ -145,6 +145,9 @@
         {% endblock request_summary_header%}
 
         {% block request_summary %}
+        {% if portfolio %}
+            {% include "includes/portfolio_request_review_steps.html" with is_editable=False domain_request=DomainRequest %}
+        {% else %}
             {% with heading_level='h3' %}
             {% with org_type=DomainRequest.get_generic_org_type_display %}
             {% include "includes/summary_item.html" with title='Type of organization' value=org_type heading_level=heading_level %}
@@ -231,6 +234,7 @@
                 </ul>
             {% endif %}
         {% endwith %}
+    {% endif %}
     {% endblock request_summary%}
     </div>
 </main>

src/registrar/templatetags/custom_filters.py

@@ -268,17 +268,17 @@ def display_requesting_entity(domain_request):
     Boise, ID
     """
     display = ""
-    if domain_request.portfolio and domain_request.organization_name == domain_request.portfolio.organization_name:
-        display = (
-            f"{domain_request.portfolio.organization_name}\n"
-            f"{domain_request.portfolio.city}, {domain_request.portfolio.state_territory}"
-        )
-    elif domain_request.sub_organization:
+    if domain_request.sub_organization:
         display = domain_request.sub_organization
-    elif domain_request.has_information_required_to_make_suborganization():
+    elif domain_request.requesting_entity_is_suborganization():
         display = (
             f"{domain_request.requested_suborganization}\n"
             f"{domain_request.suborganization_city}, {domain_request.suborganization_state_territory}"
         )
+    elif domain_request.requesting_entity_is_portfolio():
+        display = (
+            f"{domain_request.portfolio.organization_name}\n"
+            f"{domain_request.portfolio.city}, {domain_request.portfolio.state_territory}"
+        )
 
     return display

src/registrar/tests/test_views_portfolio.py

@@ -1685,9 +1685,9 @@ class TestRequestingEntity(WebTest):
         form = response.forms[0]
 
         # Test selecting an existing suborg
-        form["portfolio_requesting_entity-is_suborganization"] = True
+        form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
         form["portfolio_requesting_entity-sub_organization"] = f"{self.suborganization.id}"
-        form["portfolio_requesting_entity-is_custom_suborganization"] = False
+        form["portfolio_requesting_entity-is_requesting_new_suborganization"] = False
 
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
@@ -1718,8 +1718,8 @@ class TestRequestingEntity(WebTest):
         form = response.forms[0]
 
         # Test selecting an existing suborg
-        form["portfolio_requesting_entity-is_suborganization"] = True
-        form["portfolio_requesting_entity-is_custom_suborganization"] = True
+        form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
+        form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True
         form["portfolio_requesting_entity-sub_organization"] = ""
 
         form["portfolio_requesting_entity-requested_suborganization"] = "moon"
@@ -1759,7 +1759,7 @@ class TestRequestingEntity(WebTest):
         form = response.forms[0]
 
         # Test selecting an existing suborg
-        form["portfolio_requesting_entity-is_suborganization"] = False
+        form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = False
 
         session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
@@ -1784,7 +1784,7 @@ class TestRequestingEntity(WebTest):
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
 
         # Test missing suborganization selection
-        form["portfolio_requesting_entity-is_suborganization"] = True
+        form["portfolio_requesting_entity-requesting_entity_is_suborganization"] = True
         form["portfolio_requesting_entity-sub_organization"] = ""
 
         response = form.submit()
@@ -1792,7 +1792,7 @@ class TestRequestingEntity(WebTest):
         self.assertContains(response, "Select a suborganization.", status_code=200)
 
         # Test missing custom suborganization details
-        form["portfolio_requesting_entity-is_custom_suborganization"] = True
+        form["portfolio_requesting_entity-is_requesting_new_suborganization"] = True
         response = form.submit()
         self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
         self.assertContains(response, "Enter details for your organization name.", status_code=200)

src/registrar/views/domain_request.py

@@ -596,13 +596,13 @@ class RequestingEntity(DomainRequestWizard):
         if the user selects one."""
         requesting_entity_form = forms[1]
         cleaned_data = requesting_entity_form.cleaned_data
-        is_suborganization = cleaned_data.get("is_suborganization")
+        requesting_entity_is_suborganization = cleaned_data.get("requesting_entity_is_suborganization")
         sub_organization = cleaned_data.get("sub_organization")
         requested_suborganization = cleaned_data.get("requested_suborganization")
 
         # If no suborganization presently exists but the user filled out
         # org information then create a suborg automatically.
-        if is_suborganization and (sub_organization or requested_suborganization):
+        if requesting_entity_is_suborganization and (sub_organization or requested_suborganization):
             # Cleanup the organization name field, as this isn't for suborganizations.
             self.domain_request.organization_name = None
             self.domain_request.sub_organization = sub_organization