Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#2771 + #3015: Create requesting entity page - [MEOWARD] #2973

Merged
merged 52 commits into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
e04337f
initial form
zandercymatics Oct 17, 2024
b3e9096
dynamic options
zandercymatics Oct 17, 2024
65f1e62
add javascript logic (needs some minor refinement)
zandercymatics Oct 18, 2024
860f8f4
initial logic
zandercymatics Oct 23, 2024
888d77b
Validation logic
zandercymatics Oct 23, 2024
514cd8a
fix unlocking steps
zandercymatics Oct 23, 2024
263ec02
Add requesting entity to review page
zandercymatics Oct 23, 2024
086d197
migration
zandercymatics Oct 23, 2024
5b629bd
Delete 0134_domainrequest_requested_suborganization_and_more.py
zandercymatics Oct 23, 2024
ab7a6ac
Merge branch 'main' into za/2771-create-requesting-entity-page
zandercymatics Oct 23, 2024
0374efb
Fix merge conflict
zandercymatics Oct 23, 2024
939b11e
Fix edge cases for review page and add fields to admin
zandercymatics Oct 23, 2024
150a59c
Update portfolio_request_review_steps.html
zandercymatics Oct 23, 2024
5cb3298
Update email content, update manage button content
zandercymatics Oct 23, 2024
bde6c5e
check for org on email
zandercymatics Oct 23, 2024
5e23ebe
Fix some tests
zandercymatics Oct 24, 2024
d9ec108
fix emails
zandercymatics Oct 24, 2024
87d51c1
Fix bugs with email
zandercymatics Oct 24, 2024
178d127
linting + add fields to django admin
zandercymatics Oct 24, 2024
14bfeb7
admin show/hide logic
zandercymatics Oct 24, 2024
738ff0f
unit test stuff
zandercymatics Oct 24, 2024
d44bcad
unit tests
zandercymatics Oct 25, 2024
934d06d
lint and fix existing unit tests
zandercymatics Oct 25, 2024
0933fe4
fix weird spaces
zandercymatics Oct 25, 2024
9531076
Update src/registrar/forms/domain_request_wizard.py
zandercymatics Oct 28, 2024
7e2e75d
Merge branch 'main' into za/2771-create-requesting-entity-page
zandercymatics Oct 28, 2024
d0aff60
Update src/registrar/assets/js/get-gov-admin.js
zandercymatics Oct 30, 2024
b09e0ca
Fix bug with both org name and suborg
zandercymatics Oct 30, 2024
bc8789b
Simplify logic and use better names
zandercymatics Oct 30, 2024
ee71ba4
cleanup
zandercymatics Oct 30, 2024
b3faa00
Merge branch 'main' into za/2771-create-requesting-entity-page
zandercymatics Oct 30, 2024
a94a5b2
Readd migration after merge
zandercymatics Oct 30, 2024
c465b7f
fix bug with form save
zandercymatics Oct 30, 2024
dfb59a6
lint model
zandercymatics Oct 30, 2024
4f1febf
Update domain_request.py
zandercymatics Oct 30, 2024
bb9cb52
Cleanup bool logic to be more concise
zandercymatics Oct 30, 2024
98842c1
add comments
zandercymatics Oct 30, 2024
603e2eb
simplify javascript (a lot)
zandercymatics Oct 30, 2024
b01e707
further simplify
zandercymatics Oct 30, 2024
f0ba596
Initial logic
zandercymatics Oct 31, 2024
5109384
Add some comments
zandercymatics Oct 31, 2024
ba61ec8
Merge branch 'main' into za/2771-create-requesting-entity-page
zandercymatics Oct 31, 2024
1d34598
PR suggestions (part 1)
zandercymatics Nov 1, 2024
83720c3
Update admin.py
zandercymatics Nov 1, 2024
b05a62e
Rework readonly fields for analysts + hide when org flag is off
zandercymatics Nov 1, 2024
7cdfb7a
fix test + lint
zandercymatics Nov 1, 2024
c59289d
Update domain_request_wizard.py
zandercymatics Nov 1, 2024
7141eae
consolidate migration
zandercymatics Nov 1, 2024
a1d37ee
Update src/registrar/templates/includes/portfolio_request_review_step…
zandercymatics Nov 1, 2024
706dd4f
error messages! @abroddrick
zandercymatics Nov 1, 2024
cf0fec1
Merge branch 'za/2771-create-requesting-entity-page' of github.com:ci…
zandercymatics Nov 1, 2024
e4c15ee
Fix unit test
zandercymatics Nov 1, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/.pa11yci
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"http://localhost:8080/request/anything_else/",
"http://localhost:8080/request/requirements/",
"http://localhost:8080/request/finished/",
"http://localhost:8080/request/requesting_entity/",
"http://localhost:8080/user-profile/",
"http://localhost:8080/members/",
"http://localhost:8080/members/new-member"
Expand Down
41 changes: 40 additions & 1 deletion src/registrar/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1478,7 +1478,21 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
search_help_text = "Search by domain."

fieldsets = [
(None, {"fields": ["portfolio", "sub_organization", "creator", "domain_request", "notes"]}),
(
None,
{
"fields": [
"portfolio",
"sub_organization",
"requested_suborganization",
"suborganization_city",
"suborganization_state_territory",
"creator",
"domain_request",
"notes",
]
},
),
(".gov domain", {"fields": ["domain"]}),
("Contacts", {"fields": ["senior_official", "other_contacts", "no_other_contacts_rationale"]}),
("Background info", {"fields": ["anything_else"]}),
Expand Down Expand Up @@ -1748,6 +1762,9 @@ def status_history(self, obj):
"fields": [
"portfolio",
"sub_organization",
"requested_suborganization",
"suborganization_city",
"suborganization_state_territory",
"status_history",
"status",
"rejection_reason",
Expand Down Expand Up @@ -1868,6 +1885,28 @@ def status_history(self, obj):

change_form_template = "django/admin/domain_request_change_form.html"

# While the organization feature is under development, we can gate some fields
# from analysts for now. Remove this array and the get_fieldset overrides once this is done.
# Not my code initially, credit to Nicolle. This was once removed and like a phoenix it has been reborn.
superuser_only_fields = [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not catch this on the first pass but I think we should simply have these fields as readonly for analysts. This AC indicates so:

The Admin Readonly view for Portfolio Request, Domain info/Domain does not show the requested suborganization, suborg_city and state fields if the user is selecting a suborganization from the list. But it displays the requested suborganization, city/state fields when the user selects the "Other" option.

If that's the case, we should use pre-established patters for analyst readonly / superuser editable fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was an AC from @abroddrick which I did not list which specifies that these fields should be hidden if the organization flag is off. Adding that now

That said, I just reworked this which plays into your suggestion. I think it works better

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree read only better follows our pre-existing patterns. However, I do think design wants those fields to not show. That said, @Katherine-Osos could we have the requested suborganization, suborg_city and state fields just turn read-only when suborganization is filled in and still provide a similiar user experience for analysts?

"requested_suborganization",
"suborganization_city",
"suborganization_state_territory",
]

def get_fieldsets(self, request, obj=None):
fieldsets = super().get_fieldsets(request, obj)

# Create a modified version of fieldsets to exclude certain fields
if not request.user.has_perm("registrar.full_access_permission"):
modified_fieldsets = []
for name, data in fieldsets:
fields = data.get("fields", [])
fields = tuple(field for field in fields if field not in self.superuser_only_fields)
modified_fieldsets.append((name, {**data, "fields": fields}))
return modified_fieldsets
return fieldsets

# Trigger action when a fieldset is changed
def save_model(self, request, obj, form, change):
"""Custom save_model definition that handles edge cases"""
Expand Down
64 changes: 64 additions & 0 deletions src/registrar/assets/js/get-gov-admin.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,49 @@ function addOrRemoveSessionBoolean(name, add){

// <<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>>
// Event handlers.
/** Helper function that handles business logic for the suborganization field.
* Can be used anywhere the suborganization dropdown exists
*/
function handleSuborganizationFields(
portfolioDropdownSelector="#id_portfolio",
suborgDropdownSelector="#id_sub_organization",
requestedSuborgFieldSelector=".field-requested_suborganization",
suborgCitySelector=".field-suborganization_city",
suborgStateTerritorySelector=".field-suborganization_state_territory"
) {
// These dropdown are select2 fields so they must be interacted with via jquery
const portfolioDropdown = django.jQuery(portfolioDropdownSelector)
const suborganizationDropdown = django.jQuery(suborgDropdownSelector)
const requestedSuborgField = document.querySelector(requestedSuborgFieldSelector);
const suborgCity = document.querySelector(suborgCitySelector);
const suborgStateTerritory = document.querySelector(suborgStateTerritorySelector);
if (!suborganizationDropdown || !requestedSuborgField || !suborgCity || !suborgStateTerritory) {
console.error("Requested suborg fields not found.");
zandercymatics marked this conversation as resolved.
Show resolved Hide resolved
return;
}

function toggleSuborganizationFields() {
if (portfolioDropdown.val() && !suborganizationDropdown.val()) {
showElement(requestedSuborgField);
showElement(suborgCity);
showElement(suborgStateTerritory);
}else {
hideElement(requestedSuborgField);
hideElement(suborgCity);
hideElement(suborgStateTerritory);
}
}

// Run the function once on page startup, then attach an event listener
toggleSuborganizationFields();
suborganizationDropdown.on("change", toggleSuborganizationFields);
portfolioDropdown.on("change", toggleSuborganizationFields);
}

// <<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>><<>>
// Initialization code.


/** An IIFE for pages in DjangoAdmin that use modals.
* Dja strips out form elements, and modals generate their content outside
* of the current form scope, so we need to "inject" these inputs.
Expand Down Expand Up @@ -1170,3 +1209,28 @@ document.addEventListener('DOMContentLoaded', function() {
};
}
})();

/** An IIFE for dynamic DomainRequest fields
*/
(function dynamicDomainRequestFields(){
const domainRequestPage = document.getElementById("domainrequest_form");
if (domainRequestPage) {
handleSuborganizationFields();
}
})();


/** An IIFE for dynamic DomainInformation fields
*/
(function dynamicDomainInformationFields(){
const domainInformationPage = document.getElementById("domaininformation_form");
// DomainInformation is embedded inside domain so this should fire there too
const domainPage = document.getElementById("domain_form");
if (domainInformationPage) {
handleSuborganizationFields();
}

if (domainPage) {
handleSuborganizationFields(portfolioDropdownSelector="#id_domain_info-0-portfolio", suborgDropdownSelector="#id_domain_info-0-sub_organization");
}
})();
49 changes: 47 additions & 2 deletions src/registrar/assets/js/get-gov.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ const hideElement = (element) => {
};

/**
* Show element
*
* Show element
*
*/
const showElement = (element) => {
element.classList.remove('display-none');
zandercymatics marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -2775,3 +2775,48 @@ document.addEventListener('DOMContentLoaded', function() {
}
}
})();

/** An IIFE that intializes the requesting entity page.
* This page has a radio button that dynamically toggles some fields
* Within that, the dropdown also toggles some additional form elements.
*/
(function handleRequestingEntityFieldset() {
// Sadly, these ugly ids are the auto generated with this prefix
const formPrefix = "portfolio_requesting_entity"
const radioFieldset = document.getElementById(`id_${formPrefix}-requesting_entity_is_suborganization__fieldset`);
const radios = radioFieldset?.querySelectorAll(`input[name="${formPrefix}-requesting_entity_is_suborganization"]`);
const select = document.getElementById(`id_${formPrefix}-sub_organization`);
const suborgContainer = document.getElementById("suborganization-container");
const suborgDetailsContainer = document.getElementById("suborganization-container__details");
if (!radios || !select || !suborgContainer || !suborgDetailsContainer) return;

// requestingSuborganization: This just broadly determines if they're requesting a suborg at all
// requestingNewSuborganization: This variable determines if the user is trying to *create* a new suborganization or not.
var requestingSuborganization = Array.from(radios).find(radio => radio.checked)?.value === "True";
var requestingNewSuborganization = document.getElementById(`id_${formPrefix}-is_requesting_new_suborganization`);

function toggleSuborganization(radio=null) {
if (radio != null) requestingSuborganization = radio?.checked && radio.value === "True";
requestingSuborganization ? showElement(suborgContainer) : hideElement(suborgContainer);
requestingNewSuborganization.value = requestingSuborganization && select.value === "other" ? "True" : "False";
requestingNewSuborganization.value === "True" ? showElement(suborgDetailsContainer) : hideElement(suborgDetailsContainer);
}

// Add fake "other" option to sub_organization select
if (select && !Array.from(select.options).some(option => option.value === "other")) {
select.add(new Option("Other (enter your organization manually)", "other"));
}

if (requestingNewSuborganization.value === "True") {
select.value = "other";
}

// Add event listener to is_suborganization radio buttons, and run for initial display
toggleSuborganization();
radios.forEach(radio => {
radio.addEventListener("click", () => toggleSuborganization(radio));
});

// Add event listener to the suborg dropdown to show/hide the suborg details section
select.addEventListener("change", () => toggleSuborganization());
})();
147 changes: 144 additions & 3 deletions src/registrar/forms/domain_request_wizard.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
BaseYesNoForm,
BaseDeletableRegistrarForm,
)
from registrar.models import Contact, DomainRequest, DraftDomain, Domain, FederalAgency
from registrar.models import Contact, DomainRequest, DraftDomain, Domain, FederalAgency, Suborganization
from registrar.templatetags.url_helpers import public_site_url
from registrar.utility.enums import ValidationReturnType
from registrar.utility.constants import BranchChoices
Expand All @@ -22,10 +22,151 @@


class RequestingEntityForm(RegistrarForm):
organization_name = forms.CharField(
label="Organization name",
"""The requesting entity form contains a dropdown for suborganizations,
and some (hidden by default) input fields that allow the user to request for a suborganization.
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."""

# IMPORTANT: This is tied to DomainRequest.is_requesting_new_suborganization().
# This is due to the from_database method on DomainRequestWizard.
# Add a hidden field to store if the user is requesting a new suborganization.
# This hidden boolean is used for our javascript to communicate to us and to it.
# If true, the suborganization form will auto select a js value "Other".
# 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment: don't include line numbers in code comments please, they will fall out of date

Copy link
Contributor Author

@zandercymatics zandercymatics Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I'll include the function name instead

is_requesting_new_suborganization = forms.BooleanField(required=False, widget=forms.HiddenInput())

sub_organization = forms.ModelChoiceField(
label="Suborganization name",
required=False,
queryset=Suborganization.objects.none(),
empty_label="--Select--",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change before merging: add an error message here see response to you question here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks didn't catch that!

)
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):
"""Override of init to add the suborganization queryset"""
super().__init__(*args, **kwargs)

if self.domain_request.portfolio:
self.fields["sub_organization"].queryset = Suborganization.objects.filter(
portfolio=self.domain_request.portfolio
)

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)"""

# If it's a new suborganization, return None (equivalent to selecting nothing)
if self.cleaned_data.get("is_requesting_new_suborganization"):
return None

# 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 and "other" in suborganization:
data["portfolio_requesting_entity-sub_organization"] = ""

# Set the modified data back to the form
self.data = data

# Call the parent's full_clean method
super().full_clean()

def clean(self):
"""Custom clean implementation to handle our desired logic flow for suborganization.
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_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.")
if not cleaned_data.get("suborganization_city"):
self.add_error("suborganization_city", "Enter details for your city.")
if not cleaned_data.get("suborganization_state_territory"):
self.add_error("suborganization_state_territory", "Enter details for your state or territory.")
Copy link
Contributor

@abroddrick abroddrick Oct 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change before merging Why don't these errors match the ones you have above in the field definitions? If the others aren't triggering due to the setup, they should be removed to avoid confusion and the content copied here.

Copy link
Contributor Author

@zandercymatics zandercymatics Nov 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Just old content that evaded me I guess, the errors that get thrown are the ones in clean as these are all "optional"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Why aren't we using error definitions on the form fields instead?

elif not suborganization:
self.add_error("sub_organization", "Select a suborganization.")

return cleaned_data


class RequestingEntityYesNoForm(BaseYesNoForm):
"""The yes/no field for the RequestingEntity form."""

# This first option will change dynamically
form_choices = ((False, "Current Organization"), (True, "A suborganization. (choose from list)"))

# IMPORTANT: This is tied to DomainRequest.is_requesting_new_suborganization().
# This is due to the from_database method on DomainRequestWizard.
field_name = "requesting_entity_is_suborganization"

def __init__(self, *args, **kwargs):
"""Extend the initialization of the form from RegistrarForm __init__"""
super().__init__(*args, **kwargs)
if self.domain_request.portfolio:
self.form_choices = (
(False, self.domain_request.portfolio),
(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.
"""
# True means that the requesting entity is a suborganization,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion Key info on your return values should exist in the docstring for the function.
example to shorten it here
"""
Determines the initial checked state of the form, returning True (checked) if the requesting entity is a suborganization, and False if it is a portfolio. Returns None if neither condition is met.

"""

# whereas False means that the requesting entity is a portfolio.
if self.domain_request.requesting_entity_is_suborganization():
return True
elif self.domain_request.requesting_entity_is_portfolio():
return False
else:
return None


class OrganizationTypeForm(RegistrarForm):
Expand Down
Loading
Loading