From bc8789bc1b2e0bc8e616f6096d791269f64c071a Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 30 Oct 2024 11:22:39 -0600
Subject: [PATCH] Simplify logic and use better names
---
src/registrar/assets/js/get-gov.js | 64 ++++----
src/registrar/forms/domain_request_wizard.py | 57 ++++---
src/registrar/models/domain_request.py | 74 ++++-----
.../domain_request_requesting_entity.html | 10 +-
.../templates/domain_request_status.html | 6 +-
.../portfolio_request_review_steps.html | 28 ++--
.../includes/portfolio_status_manage.html | 7 -
.../includes/request_status_manage.html | 144 +++++++++---------
src/registrar/templatetags/custom_filters.py | 14 +-
src/registrar/tests/test_views_portfolio.py | 14 +-
src/registrar/views/domain_request.py | 4 +-
11 files changed, 205 insertions(+), 217 deletions(-)
delete mode 100644 src/registrar/templates/includes/portfolio_status_manage.html
diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 6302e57f7..6182bc828 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -2741,49 +2741,45 @@ document.addEventListener('DOMContentLoaded', function() {
* This page has a radio button that dynamically toggles some fields
* Within that, the dropdown also toggles some additional form elements.
*/
-(function handleRequestingEntityFieldset() {
- // Check if the requesting-entity-fieldset exists.
+(function handleRequestingEntityFieldset() {
+ // 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
- if (subOrgSelect.value === "other") {
- showElement(suborganizationDetailsFieldset);
- isCustomSuborganization.value = "True";
+ // Handle custom suborganizations
+ if (subOrgSelect.value === "other") {
+ showElement(suborganizationDetailsContainer);
+ isRequestingSuborganization.value = "True";
+ } else {
+ hideElement(suborganizationDetailsContainer);
+ isRequestingSuborganization.value = "False";
+ }
} else {
- hideElement(suborganizationDetailsFieldset);
- isCustomSuborganization.value = "False";
+ 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";
+ }
});
})();
diff --git a/src/registrar/forms/domain_request_wizard.py b/src/registrar/forms/domain_request_wizard.py
index 255eff332..e6188eb33 100644
--- a/src/registrar/forms/domain_request_wizard.py
+++ b/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,23 +73,25 @@ def __init__(self, *args, **kwargs):
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
- data["portfolio_requesting_entity-sub_organization"] = ""
+ if suborganization and "other" in suborganization:
+ data["portfolio_requesting_entity-sub_organization"] = ""
# Set the modified data back to the form
self.data = data
@@ -110,11 +104,16 @@ def clean(self):
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 @@ def clean(self):
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 @@ def form_is_checked(self):
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
diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py
index 8ac12e085..a04cea526 100644
--- a/src/registrar/models/domain_request.py
+++ b/src/registrar/models/domain_request.py
@@ -1125,16 +1125,49 @@ def reject_with_prejudice(self):
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 ###
+ # ## Form policies ## #
#
# These methods control what questions need to be answered by applicants
# during the domain request flow. They are policies about the domain request so
@@ -1203,39 +1236,6 @@ def is_federal(self) -> Union[bool, None]:
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
diff --git a/src/registrar/templates/domain_request_requesting_entity.html b/src/registrar/templates/domain_request_requesting_entity.html
index 04ceb4a1a..d09e8ab89 100644
--- a/src/registrar/templates/domain_request_requesting_entity.html
+++ b/src/registrar/templates/domain_request_requesting_entity.html
@@ -7,7 +7,7 @@
{% endblock %}
{% block form_fields %}
-