-
Notifications
You must be signed in to change notification settings - Fork 19
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
Ticket #2484: Dynamic portfolio fields #2548
Changes from 12 commits
1d6981f
b092fc6
a2f87c7
e90ef9b
ee7b886
ac6561d
32630ad
ea47a7a
47779f8
44b8163
0feb0a7
b89ae53
413bb53
8081e92
2c4c9dd
67cd368
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -511,11 +511,21 @@ function initializeWidgetOnList(list, parentId) { | |
var actionNeededReasonDropdown = document.querySelector("#id_action_needed_reason"); | ||
var actionNeededEmail = document.querySelector("#id_action_needed_reason_email"); | ||
var readonlyView = document.querySelector("#action-needed-reason-email-readonly"); | ||
if(!actionNeededEmail || !actionNeededReasonDropdown) { | ||
return; | ||
} | ||
|
||
let emailWasSent = document.getElementById("action-needed-email-sent"); | ||
if (!emailWasSent) { | ||
return; | ||
} | ||
|
||
let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; | ||
let actionNeededEmailsJson = JSON.parse(actionNeededEmailData); | ||
if(!actionNeededEmailData) { | ||
return; | ||
} | ||
|
||
let actionNeededEmailsJson = JSON.parse(actionNeededEmailData); | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null | ||
const emailSentSessionVariableName = `actionNeededEmailSent-${domainRequestId}`; | ||
const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; | ||
|
@@ -750,3 +760,113 @@ function initializeWidgetOnList(list, parentId) { | |
}); | ||
} | ||
})(); | ||
|
||
|
||
/** An IIFE for copy summary button (appears in DomainRegistry models) | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
(function dynamicPortfolioFields(){ | ||
|
||
document.addEventListener('DOMContentLoaded', function() { | ||
|
||
let isPortfolioPage = document.querySelector("#portfolio_form"); | ||
if (!isPortfolioPage) { | ||
return; | ||
} | ||
|
||
// $ symbolically denotes that this is using jQuery | ||
let $federalAgency = django.jQuery("#id_federal_agency"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Q: Why use django.JQuery instead of getElement or querySelector? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, its because everywhere else in the app federal agency is a select2 (autocomplete field). The portfolio page is still undergoing some work - so its likely that this will be added to that as well. Doing it this way gives us out of the box support so we don't have to alter this script when we do. For context: you can't do .value on Select2 objects. You can only manipulate them through jQuery, its a quirk of that particular library. Went down that rabbit hole a few tickets ago |
||
let organizationType = document.querySelector("#id_organization_type"); | ||
rachidatecs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if ($federalAgency && organizationType) { | ||
// Execute this function once on load | ||
handleFederalAgencyChange($federalAgency, organizationType); | ||
|
||
// Attach the change event listener | ||
$federalAgency.on("change", function() { | ||
handleFederalAgencyChange($federalAgency, organizationType); | ||
}); | ||
} | ||
|
||
// Handle dynamically hiding the urbanization field | ||
let urbanizationField = document.querySelector(".field-urbanization"); | ||
let stateTerritory = document.querySelector("#id_state_territory"); | ||
if (urbanizationField && stateTerritory) { | ||
// Execute this function once on load | ||
handleStateTerritoryChange(stateTerritory, urbanizationField); | ||
|
||
// Attach the change event listener for state/territory | ||
stateTerritory.addEventListener("change", function() { | ||
handleStateTerritoryChange(stateTerritory, urbanizationField); | ||
}); | ||
} | ||
}); | ||
|
||
function handleFederalAgencyChange(federalAgency, organizationType) { | ||
// Set the org type to federal if an agency is selected | ||
let selectedText = federalAgency.find("option:selected").text(); | ||
if (selectedText !== "Non-Federal Agency" && selectedText) { | ||
if (organizationType.value !== "federal") { | ||
organizationType.value = "federal"; | ||
} | ||
}else if (selectedText === "Non-Federal Agency" && organizationType.value === "federal") { | ||
organizationType.value = ""; | ||
} | ||
|
||
// There isn't a federal senior official associated with null records and non federal agencies | ||
if (!selectedText || selectedText === "Non-Federal Agency") { | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return; | ||
} | ||
|
||
// Get the associated senior official with this federal agency | ||
let $seniorOfficial = django.jQuery("#id_senior_official"); | ||
if (!$seniorOfficial) { | ||
console.log("Could not find the senior official field"); | ||
return; | ||
} | ||
|
||
let seniorOfficialApi = document.querySelector("#senior_official_from_agency_json_url").value; | ||
fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) | ||
.then(response => { | ||
const statusCode = response.status; | ||
return response.json().then(data => ({ statusCode, data })); | ||
}) | ||
.then(({ statusCode, data }) => { | ||
if (data.error) { | ||
// Clear the field if the SO doesn't exist. | ||
if (statusCode === 404) { | ||
$seniorOfficial.val("").trigger("change"); | ||
} | ||
console.error("Error in AJAX call: " + data.error); | ||
return; | ||
} | ||
|
||
let seniorOfficialId = data.id; | ||
let seniorOfficialName = [data.first_name, data.last_name].join(" "); | ||
if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ | ||
// Clear the field if the SO doesn't exist | ||
$seniorOfficial.val("").trigger("change"); | ||
return; | ||
} | ||
|
||
// Add the senior official to the dropdown. | ||
// This format supports select2 - if we decide to convert this field in the future. | ||
if ($seniorOfficial.find(`option[value='${seniorOfficialId}']`).length) { | ||
// Select the value that is associated with the current Senior Official. | ||
$seniorOfficial.val(seniorOfficialId).trigger("change"); | ||
} else { | ||
// Create a DOM Option that matches the desired Senior Official. Then append it and select it. | ||
let userOption = new Option(seniorOfficialName, seniorOfficialId, true, true); | ||
$seniorOfficial.append(userOption).trigger("change"); | ||
} | ||
}) | ||
.catch(error => console.error("Error fetching senior official: ", error)); | ||
} | ||
|
||
function handleStateTerritoryChange(stateTerritory, urbanizationField) { | ||
let selectedValue = stateTerritory.value; | ||
if (selectedValue === "PR") { | ||
showElement(urbanizationField) | ||
} else { | ||
hideElement(urbanizationField) | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
})(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -110,3 +110,12 @@ class Portfolio(TimeStampedModel): | |
|
||
def __str__(self) -> str: | ||
return f"{self.organization_name}" | ||
|
||
def save(self, *args, **kwargs): | ||
"""Save override for custom properties""" | ||
|
||
# The urbanization field is only intended for the state_territory puerto rico | ||
if self.state_territory != self.StateTerritoryChoices.PUERTO_RICO and self.urbanization: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs a unit test |
||
self.urbanization = None | ||
|
||
super().save(*args, **kwargs) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
from django.urls import reverse | ||
from django.test import TestCase, Client | ||
from registrar.models import FederalAgency, SeniorOfficial, User | ||
from django.contrib.auth import get_user_model | ||
from registrar.tests.common import create_superuser, create_user | ||
|
||
|
||
class GetSeniorOfficialJsonTest(TestCase): | ||
def setUp(self): | ||
self.client = Client() | ||
p = "password" | ||
self.user = get_user_model().objects.create_user(username="testuser", password=p) | ||
|
||
self.superuser = create_superuser() | ||
self.analyst_user = create_user() | ||
|
||
self.agency = FederalAgency.objects.create(agency="Test Agency") | ||
self.senior_official = SeniorOfficial.objects.create( | ||
first_name="John", last_name="Doe", title="Director", federal_agency=self.agency | ||
) | ||
|
||
self.api_url = reverse("get-senior-official-from-federal-agency-json") | ||
|
||
def tearDown(self): | ||
User.objects.all().delete() | ||
SeniorOfficial.objects.all().delete() | ||
FederalAgency.objects.all().delete() | ||
|
||
def test_get_senior_official_json_authenticated_superuser(self): | ||
"""Test that a superuser can fetch the senior official information.""" | ||
p = "adminpass" | ||
self.client.login(username="superuser", password=p) | ||
response = self.client.get(self.api_url, {"agency_name": "Test Agency"}) | ||
self.assertEqual(response.status_code, 200) | ||
data = response.json() | ||
self.assertEqual(data["id"], self.senior_official.id) | ||
self.assertEqual(data["first_name"], "John") | ||
self.assertEqual(data["last_name"], "Doe") | ||
self.assertEqual(data["title"], "Director") | ||
|
||
def test_get_senior_official_json_authenticated_analyst(self): | ||
"""Test that an analyst user can fetch the senior official's information.""" | ||
p = "userpass" | ||
self.client.login(username="staffuser", password=p) | ||
response = self.client.get(self.api_url, {"agency_name": "Test Agency"}) | ||
self.assertEqual(response.status_code, 200) | ||
data = response.json() | ||
self.assertEqual(data["id"], self.senior_official.id) | ||
self.assertEqual(data["first_name"], "John") | ||
self.assertEqual(data["last_name"], "Doe") | ||
self.assertEqual(data["title"], "Director") | ||
|
||
def test_get_senior_official_json_unauthenticated(self): | ||
"""Test that an unauthenticated user receives a 403 with an error message.""" | ||
p = "password" | ||
self.client.login(username="testuser", password=p) | ||
response = self.client.get(self.api_url, {"agency_name": "Test Agency"}) | ||
self.assertEqual(response.status_code, 302) | ||
|
||
def test_get_senior_official_json_not_found(self): | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
"""Test that a request for a non-existent agency returns a 404 with an error message.""" | ||
p = "adminpass" | ||
self.client.login(username="superuser", password=p) | ||
response = self.client.get(self.api_url, {"agency_name": "Non-Federal Agency"}) | ||
self.assertEqual(response.status_code, 404) | ||
data = response.json() | ||
self.assertEqual(data["error"], "Senior Official not found") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
import logging | ||
from django.http import JsonResponse | ||
from django.forms.models import model_to_dict | ||
from registrar.models import FederalAgency, SeniorOfficial | ||
from django.contrib.admin.views.decorators import staff_member_required | ||
from django.contrib.auth.decorators import login_required | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
@login_required | ||
@staff_member_required | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems redundant with the superuser and analyst checks you're doing below. That's not a bad thing, but I'm wondering why you bothered with the decorator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This decorator redirects you to the admin login page and basically does the default django checks for admin perms. Tiny QOL thing, otherwise you do get blocked but you can technically still """access""" the endpoint |
||
def get_senior_official_from_federal_agency_json(request): | ||
"""Returns federal_agency information as a JSON""" | ||
|
||
# This API is only accessible to admins and analysts | ||
superuser_perm = request.user.has_perm("registrar.full_access_permission") | ||
analyst_perm = request.user.has_perm("registrar.analyst_access_permission") | ||
if not request.user.is_authenticated or not any([analyst_perm, superuser_perm]): | ||
return JsonResponse({"error": "You do not have access to this resource"}, status=403) | ||
zandercymatics marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
agency_name = request.GET.get("agency_name") | ||
agency = FederalAgency.objects.filter(agency=agency_name).first() | ||
senior_official = SeniorOfficial.objects.filter(federal_agency=agency).first() | ||
if agency and senior_official: | ||
# Convert the agency object to a dictionary | ||
so_dict = model_to_dict(senior_official) | ||
|
||
# The phone number field isn't json serializable, so we | ||
# convert this to a string first if it exists. | ||
if "phone" in so_dict and so_dict.get("phone"): | ||
so_dict["phone"] = str(so_dict["phone"]) | ||
|
||
return JsonResponse(so_dict) | ||
else: | ||
return JsonResponse({"error": "Senior Official not found"}, status=404) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 534 should be handling this and !actionNeededEmail || !actionNeededReasonDropdown cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added these because this script was failing on the portfolio page, causing script execution to stop (on textContent because the query returned none). That said I can simplify these checks to just check on if action-needed-emails-data exists or not before trying to do .textContent