From 1d6981f282dff84f8be8a2a83bf5b981719da750 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 7 Aug 2024 10:43:09 -0600 Subject: [PATCH 01/15] Update get-gov-admin.js --- src/registrar/assets/js/get-gov-admin.js | 81 ++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 04f5417b0..e5871eedc 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -750,3 +750,84 @@ function initializeWidgetOnList(list, parentId) { }); } })(); + + +/** An IIFE for copy summary button (appears in DomainRegistry models) +*/ +(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"); + let organizationType = document.querySelector("#id_organization_type"); + 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) { + organizationType.value = "federal"; + }else { + if (organizationType.value === "federal") { + organizationType.value = ""; + } + } + + // Automatically set the SO field. + // How do we do this without an API?????????? + // let seniorOfficialId = assignSelfButton.getAttribute("data-user-id"); + // let seniorOfficialName = assignSelfButton.getAttribute("data-user-name"); + // if (!currentUserId || !currentUserName){ + // console.error("Could not assign current user: no values found.") + // return; + // } + + // if (selector.find(`option[value='${currentUserId}']`).length) { + // // Select the value that is associated with the current user. + // selector.val(currentUserId).trigger("change"); + // } else { + // // Create a DOM Option that matches the desired user. Then append it and select it. + // let userOption = new Option(currentUserName, currentUserId, true, true); + // selector.append(userOption).trigger("change"); + // } + + } + + function handleStateTerritoryChange(stateTerritory, urbanizationField) { + let selectedValue = stateTerritory.value; + if (selectedValue === "PR") { + showElement(urbanizationField) + } else { + hideElement(urbanizationField) + } + } +})(); \ No newline at end of file From a2f87c7084033fec19f7e546e9c03fdabc4d5b6f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 09:17:28 -0600 Subject: [PATCH 02/15] Add API to select SO information --- src/registrar/assets/js/get-gov-admin.js | 79 +++++++++++++------ src/registrar/config/urls.py | 7 ++ .../django/admin/portfolio_change_form.html | 7 ++ src/registrar/views/utility/__init__.py | 1 + src/registrar/views/utility/api_views.py | 32 ++++++++ 5 files changed, 103 insertions(+), 23 deletions(-) create mode 100644 src/registrar/views/utility/api_views.py diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e5871eedc..2ce8d94da 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -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"); - let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; - let actionNeededEmailsJson = JSON.parse(actionNeededEmailData); + if (!emailWasSent) { + return; + } + let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; + if(!actionNeededEmailData) { + return; + } + + let actionNeededEmailsJson = JSON.parse(actionNeededEmailData); const domainRequestId = actionNeededReasonDropdown ? document.querySelector("#domain_request_id").value : null const emailSentSessionVariableName = `actionNeededEmailSent-${domainRequestId}`; const oldDropdownValue = actionNeededReasonDropdown ? actionNeededReasonDropdown.value : null; @@ -791,35 +801,58 @@ function initializeWidgetOnList(list, parentId) { }); 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) { - organizationType.value = "federal"; - }else { - if (organizationType.value === "federal") { - organizationType.value = ""; + if (organizationType.value !== "federal") { + organizationType.value = "federal"; } + // Set the SO field + }else if (selectedText === "Non-Federal Agency" && organizationType.value === "federal") { + organizationType.value = ""; + // Set the SO field + } + + // There isn't a senior official associated with null records and non federal agencies + if (!selectedText || selectedText === "Non-Federal Agency") { + return; } - // Automatically set the SO field. - // How do we do this without an API?????????? - // let seniorOfficialId = assignSelfButton.getAttribute("data-user-id"); - // let seniorOfficialName = assignSelfButton.getAttribute("data-user-name"); - // if (!currentUserId || !currentUserName){ - // console.error("Could not assign current user: no values found.") - // 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; + } - // if (selector.find(`option[value='${currentUserId}']`).length) { - // // Select the value that is associated with the current user. - // selector.val(currentUserId).trigger("change"); - // } else { - // // Create a DOM Option that matches the desired user. Then append it and select it. - // let userOption = new Option(currentUserName, currentUserId, true, true); - // selector.append(userOption).trigger("change"); - // } + let seniorOfficialApi = document.querySelector("#senior_official_from_agency_json_url").value; + fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) + .then(response => response.json()) + .then(data => { + if (data.error) { + 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()){ + console.error("Could not assign current Senior Official: no values found.") + 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) { diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 90137c4af..4bf12894b 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -24,6 +24,7 @@ from registrar.views.report_views import ( from registrar.views.domain_request import Step from registrar.views.domain_requests_json import get_domain_requests_json +from registrar.views.utility.api_views import get_senior_official_from_federal_agency_json from registrar.views.domains_json import get_domains_json from registrar.views.utility import always_404 from api.views import available, get_current_federal, get_current_full @@ -155,6 +156,12 @@ urlpatterns = [ path("api/v1/available/", available, name="available"), path("api/v1/get-report/current-federal", get_current_federal, name="get-current-federal"), path("api/v1/get-report/current-full", get_current_full, name="get-current-full"), + # TODO convert to admin view + path( + "api/v1/get-senior-official-from-federal-agency-json/", + get_senior_official_from_federal_agency_json, + name="get-senior-official-from-federal-agency-json" + ), path( "todo", lambda r: always_404(r, "We forgot to include this link, sorry."), diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 3a6f13ccf..65020cc0a 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -1,6 +1,13 @@ {% extends 'django/admin/email_clipboard_change_form.html' %} {% load i18n static %} +{% block content %} + {% comment %} Stores the json endpoint in a url for easlier access {% endcomment %} + {% url 'get-senior-official-from-federal-agency-json' as url %} + + {{ block.super }} +{% endblock content %} + {% block after_related_objects %}

Associated groups and suborganizations

diff --git a/src/registrar/views/utility/__init__.py b/src/registrar/views/utility/__init__.py index 7299c0368..7219f4358 100644 --- a/src/registrar/views/utility/__init__.py +++ b/src/registrar/views/utility/__init__.py @@ -8,3 +8,4 @@ from .permission_views import ( DomainInvitationPermissionDeleteView, DomainRequestWizardPermissionView, ) +from .api_views import get_senior_official_from_federal_agency_json diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py new file mode 100644 index 000000000..6bd349d0a --- /dev/null +++ b/src/registrar/views/utility/api_views.py @@ -0,0 +1,32 @@ +import logging +from django.http import JsonResponse +from django.forms.models import model_to_dict +from registrar.models import FederalAgency, SeniorOfficial +from django.utils.dateformat import format +from django.contrib.auth.decorators import login_required +from django.urls import reverse +from django.db.models import Q + +logger = logging.getLogger(__name__) + + +@login_required +def get_senior_official_from_federal_agency_json(request): + """Returns federal_agency information as a JSON""" + + # This API is only accessible to admins + 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 analyst_perm or not superuser_perm: + # We intentionally don't return anything here + return {} + + 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) + return JsonResponse(so_dict) + else: + return JsonResponse({"error": "Senior Official not found"}) \ No newline at end of file From e90ef9ba98b96af2a07d169889eb27d117df8928 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 09:53:37 -0600 Subject: [PATCH 03/15] Add unit tests for api --- src/registrar/assets/js/get-gov-admin.js | 2 +- src/registrar/config/urls.py | 11 ++-- src/registrar/tests/test_api.py | 64 ++++++++++++++++++++++++ src/registrar/views/utility/api_views.py | 7 ++- 4 files changed, 73 insertions(+), 11 deletions(-) create mode 100644 src/registrar/tests/test_api.py diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 2ce8d94da..277f81b72 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -863,4 +863,4 @@ function initializeWidgetOnList(list, parentId) { hideElement(urbanizationField) } } -})(); \ No newline at end of file +})(); diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 4bf12894b..b58ff0d33 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -124,6 +124,11 @@ urlpatterns = [ AnalyticsView.as_view(), name="analytics", ), + path( + "admin/api/get-senior-official-from-federal-agency-json/", + get_senior_official_from_federal_agency_json, + name="get-senior-official-from-federal-agency-json" + ), path("admin/", admin.site.urls), path( "reports/export_data_type_user/", @@ -156,12 +161,6 @@ urlpatterns = [ path("api/v1/available/", available, name="available"), path("api/v1/get-report/current-federal", get_current_federal, name="get-current-federal"), path("api/v1/get-report/current-full", get_current_full, name="get-current-full"), - # TODO convert to admin view - path( - "api/v1/get-senior-official-from-federal-agency-json/", - get_senior_official_from_federal_agency_json, - name="get-senior-official-from-federal-agency-json" - ), path( "todo", lambda r: always_404(r, "We forgot to include this link, sorry."), diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py new file mode 100644 index 000000000..3eb7453f1 --- /dev/null +++ b/src/registrar/tests/test_api.py @@ -0,0 +1,64 @@ +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() + self.user = get_user_model().objects.create_user(username="testuser", password="password") + + 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.""" + self.client.login(username="superuser", password="adminpass") + 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.""" + self.client.login(username="staffuser", password="userpass") + 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.""" + self.client.login(username="testuser", password="password") + response = self.client.get(self.api_url, {"agency_name": "Test Agency"}) + self.assertEqual(response.status_code, 403) + data = response.json() + self.assertEqual(data["error"], "You do not have access to this resource") + + def test_get_senior_official_json_not_found(self): + """Test that a request for a non-existent agency returns a 404 with an error message.""" + self.client.login(username="superuser", password="adminpass") + 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") diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 6bd349d0a..3b9c6f54c 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -14,12 +14,11 @@ logger = logging.getLogger(__name__) def get_senior_official_from_federal_agency_json(request): """Returns federal_agency information as a JSON""" - # This API is only accessible to admins + # 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 analyst_perm or not superuser_perm: - # We intentionally don't return anything here - return {} + return JsonResponse({"error": "You do not have access to this resource"}, status=403) agency_name = request.GET.get("agency_name") agency = FederalAgency.objects.filter(agency=agency_name).first() @@ -29,4 +28,4 @@ def get_senior_official_from_federal_agency_json(request): so_dict = model_to_dict(senior_official) return JsonResponse(so_dict) else: - return JsonResponse({"error": "Senior Official not found"}) \ No newline at end of file + return JsonResponse({"error": "Senior Official not found"}, status=404) From ee7b886fbc5528123fadaf36fe5e5f66ad64c86a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 10:21:10 -0600 Subject: [PATCH 04/15] Fix perms --- src/registrar/views/utility/api_views.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index 3b9c6f54c..a1639529f 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -2,22 +2,21 @@ import logging from django.http import JsonResponse from django.forms.models import model_to_dict from registrar.models import FederalAgency, SeniorOfficial -from django.utils.dateformat import format +from django.contrib.admin.views.decorators import staff_member_required from django.contrib.auth.decorators import login_required -from django.urls import reverse -from django.db.models import Q logger = logging.getLogger(__name__) @login_required +@staff_member_required 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 analyst_perm or not superuser_perm: + 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) agency_name = request.GET.get("agency_name") From ac6561d879257c757d6b6a49194a328edd39d0cd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 10:23:09 -0600 Subject: [PATCH 05/15] Linting --- src/registrar/config/urls.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index b58ff0d33..4635d319f 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -125,9 +125,9 @@ urlpatterns = [ name="analytics", ), path( - "admin/api/get-senior-official-from-federal-agency-json/", - get_senior_official_from_federal_agency_json, - name="get-senior-official-from-federal-agency-json" + "admin/api/get-senior-official-from-federal-agency-json/", + get_senior_official_from_federal_agency_json, + name="get-senior-official-from-federal-agency-json", ), path("admin/", admin.site.urls), path( From 32630ad39a624c03a1f10012a4af45123e9af96f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 10:32:27 -0600 Subject: [PATCH 06/15] Cleanup tests --- src/registrar/tests/test_api.py | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index 3eb7453f1..fe4ee49e9 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -27,7 +27,8 @@ class GetSeniorOfficialJsonTest(TestCase): def test_get_senior_official_json_authenticated_superuser(self): """Test that a superuser can fetch the senior official information.""" - self.client.login(username="superuser", password="adminpass") + 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() @@ -38,7 +39,8 @@ class GetSeniorOfficialJsonTest(TestCase): def test_get_senior_official_json_authenticated_analyst(self): """Test that an analyst user can fetch the senior official's information.""" - self.client.login(username="staffuser", password="userpass") + 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() @@ -49,15 +51,15 @@ class GetSeniorOfficialJsonTest(TestCase): def test_get_senior_official_json_unauthenticated(self): """Test that an unauthenticated user receives a 403 with an error message.""" - self.client.login(username="testuser", password="password") + 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, 403) - data = response.json() - self.assertEqual(data["error"], "You do not have access to this resource") + self.assertEqual(response.status_code, 302) def test_get_senior_official_json_not_found(self): """Test that a request for a non-existent agency returns a 404 with an error message.""" - self.client.login(username="superuser", password="adminpass") + 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() From ea47a7a604e5e17e095ba2eadf7da28e545402b4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 8 Aug 2024 10:39:12 -0600 Subject: [PATCH 07/15] Update test_api.py --- src/registrar/tests/test_api.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py index fe4ee49e9..0025bc902 100644 --- a/src/registrar/tests/test_api.py +++ b/src/registrar/tests/test_api.py @@ -8,7 +8,8 @@ from registrar.tests.common import create_superuser, create_user class GetSeniorOfficialJsonTest(TestCase): def setUp(self): self.client = Client() - self.user = get_user_model().objects.create_user(username="testuser", password="password") + p = "password" + self.user = get_user_model().objects.create_user(username="testuser", password=p) self.superuser = create_superuser() self.analyst_user = create_user() From 47779f8f2974a9cafe1bf289095f5bce2377d6d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:20:37 -0600 Subject: [PATCH 08/15] Update src/registrar/templates/django/admin/portfolio_change_form.html Co-authored-by: Matt-Spence --- src/registrar/templates/django/admin/portfolio_change_form.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/portfolio_change_form.html b/src/registrar/templates/django/admin/portfolio_change_form.html index 65020cc0a..f6382758b 100644 --- a/src/registrar/templates/django/admin/portfolio_change_form.html +++ b/src/registrar/templates/django/admin/portfolio_change_form.html @@ -2,7 +2,7 @@ {% load i18n static %} {% block content %} - {% comment %} Stores the json endpoint in a url for easlier access {% endcomment %} + {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} {% url 'get-senior-official-from-federal-agency-json' as url %} {{ block.super }} From 44b816342276649014d43bd2795da4bff016c081 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:44:25 -0600 Subject: [PATCH 09/15] Add PR suggestions --- src/registrar/assets/js/get-gov-admin.js | 7 ++++--- src/registrar/models/portfolio.py | 9 +++++++++ src/registrar/views/utility/api_views.py | 6 ++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 277f81b72..f8bba4967 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -807,10 +807,8 @@ function initializeWidgetOnList(list, parentId) { if (organizationType.value !== "federal") { organizationType.value = "federal"; } - // Set the SO field }else if (selectedText === "Non-Federal Agency" && organizationType.value === "federal") { organizationType.value = ""; - // Set the SO field } // There isn't a senior official associated with null records and non federal agencies @@ -830,6 +828,8 @@ function initializeWidgetOnList(list, parentId) { .then(response => response.json()) .then(data => { if (data.error) { + // Clear the field if the SO doesn't exist + $seniorOfficial.val("").trigger("change"); console.error('Error in AJAX call: ' + data.error); return; } @@ -837,7 +837,8 @@ function initializeWidgetOnList(list, parentId) { let seniorOfficialId = data.id; let seniorOfficialName = [data.first_name, data.last_name].join(" "); if (!seniorOfficialId || !seniorOfficialName || !seniorOfficialName.trim()){ - console.error("Could not assign current Senior Official: no values found.") + // Clear the field if the SO doesn't exist + $seniorOfficial.val("").trigger("change"); return; } diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index 06b01e672..e705aaa83 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -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""" + + # We can't have urbanization if the state isn't puerto rico + if self.state_territory != self.StateTerritoryChoices.PUERTO_RICO and self.urbanization: + self.urbanization = None + + super().save(*args, **kwargs) diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index a1639529f..f1cb7c75f 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -25,6 +25,12 @@ def get_senior_official_from_federal_agency_json(request): 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) From 0feb0a7ef8b8f158d6bc6ef4251daa462f143537 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 08:45:58 -0600 Subject: [PATCH 10/15] Linting and cleanup --- src/registrar/assets/js/get-gov-admin.js | 4 ++-- src/registrar/views/utility/api_views.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index f8bba4967..1f5ab6734 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -830,7 +830,7 @@ function initializeWidgetOnList(list, parentId) { if (data.error) { // Clear the field if the SO doesn't exist $seniorOfficial.val("").trigger("change"); - console.error('Error in AJAX call: ' + data.error); + console.error("Error in AJAX call: " + data.error); return; } @@ -853,7 +853,7 @@ function initializeWidgetOnList(list, parentId) { $seniorOfficial.append(userOption).trigger("change"); } }) - .catch(error => console.error('Error fetching senior official:', error)); + .catch(error => console.error("Error fetching senior official: ", error)); } function handleStateTerritoryChange(stateTerritory, urbanizationField) { diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py index f1cb7c75f..2c9414d1d 100644 --- a/src/registrar/views/utility/api_views.py +++ b/src/registrar/views/utility/api_views.py @@ -26,7 +26,7 @@ def get_senior_official_from_federal_agency_json(request): # Convert the agency object to a dictionary so_dict = model_to_dict(senior_official) - # The phone number field isn't json serializable, so we + # 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"]) From b89ae53c84679fa72d6e41852f64f38f80f3fd72 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 12 Aug 2024 09:11:14 -0600 Subject: [PATCH 11/15] Check for api response codes when resetting values --- src/registrar/assets/js/get-gov-admin.js | 17 +++++++++++------ src/registrar/models/portfolio.py | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index 1f5ab6734..e2fb8e953 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -810,8 +810,8 @@ function initializeWidgetOnList(list, parentId) { }else if (selectedText === "Non-Federal Agency" && organizationType.value === "federal") { organizationType.value = ""; } - - // There isn't a senior official associated with null records and non federal agencies + + // There isn't a federal senior official associated with null records and non federal agencies if (!selectedText || selectedText === "Non-Federal Agency") { return; } @@ -825,11 +825,16 @@ function initializeWidgetOnList(list, parentId) { let seniorOfficialApi = document.querySelector("#senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) - .then(response => response.json()) - .then(data => { + .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 - $seniorOfficial.val("").trigger("change"); + // 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; } diff --git a/src/registrar/models/portfolio.py b/src/registrar/models/portfolio.py index e705aaa83..484c45e8c 100644 --- a/src/registrar/models/portfolio.py +++ b/src/registrar/models/portfolio.py @@ -114,7 +114,7 @@ class Portfolio(TimeStampedModel): def save(self, *args, **kwargs): """Save override for custom properties""" - # We can't have urbanization if the state isn't puerto rico + # The urbanization field is only intended for the state_territory puerto rico if self.state_territory != self.StateTerritoryChoices.PUERTO_RICO and self.urbanization: self.urbanization = None From 413bb53664127c6c5c0d168061484f99b7ae5817 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Aug 2024 10:53:57 -0600 Subject: [PATCH 12/15] PR suggestions (minus unit test) --- src/registrar/assets/js/get-gov-admin.js | 42 +++++++++++++----------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js index e2fb8e953..93b8359bf 100644 --- a/src/registrar/assets/js/get-gov-admin.js +++ b/src/registrar/assets/js/get-gov-admin.js @@ -511,16 +511,15 @@ 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) { + + let emailData = document.getElementById('action-needed-emails-data'); + if (!emailData) { return; } - let actionNeededEmailData = document.getElementById('action-needed-emails-data').textContent; + let actionNeededEmailData = emailData.textContent; if(!actionNeededEmailData) { return; } @@ -762,20 +761,20 @@ function initializeWidgetOnList(list, parentId) { })(); -/** An IIFE for copy summary button (appears in DomainRegistry models) +/** An IIFE for dynamically changing some fields on the portfolio admin model */ (function dynamicPortfolioFields(){ document.addEventListener('DOMContentLoaded', function() { - let isPortfolioPage = document.querySelector("#portfolio_form"); + let isPortfolioPage = document.getElementById("portfolio_form"); if (!isPortfolioPage) { return; } // $ symbolically denotes that this is using jQuery let $federalAgency = django.jQuery("#id_federal_agency"); - let organizationType = document.querySelector("#id_organization_type"); + let organizationType = document.getElementById("id_organization_type"); if ($federalAgency && organizationType) { // Execute this function once on load handleFederalAgencyChange($federalAgency, organizationType); @@ -788,7 +787,7 @@ function initializeWidgetOnList(list, parentId) { // Handle dynamically hiding the urbanization field let urbanizationField = document.querySelector(".field-urbanization"); - let stateTerritory = document.querySelector("#id_state_territory"); + let stateTerritory = document.getElementById("id_state_territory"); if (urbanizationField && stateTerritory) { // Execute this function once on load handleStateTerritoryChange(stateTerritory, urbanizationField); @@ -803,17 +802,20 @@ function initializeWidgetOnList(list, parentId) { 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) { + + // There isn't a federal senior official associated with null records + if (!selectedText) { + return; + } + + if (selectedText !== "Non-Federal Agency") { 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") { - return; + }else { + if (organizationType.value === "federal") { + organizationType.value = ""; + } } // Get the associated senior official with this federal agency @@ -823,7 +825,7 @@ function initializeWidgetOnList(list, parentId) { return; } - let seniorOfficialApi = document.querySelector("#senior_official_from_agency_json_url").value; + let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value; fetch(`${seniorOfficialApi}?agency_name=${selectedText}`) .then(response => { const statusCode = response.status; @@ -834,8 +836,10 @@ function initializeWidgetOnList(list, parentId) { // Clear the field if the SO doesn't exist. if (statusCode === 404) { $seniorOfficial.val("").trigger("change"); + console.warn("Record not found: " + data.error); + }else { + console.error("Error in AJAX call: " + data.error); } - console.error("Error in AJAX call: " + data.error); return; } From 8081e9252f8022bca81c3b52beb9ff7ae668d995 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:10:10 -0600 Subject: [PATCH 13/15] PR suggestions - unit test --- src/registrar/tests/test_models.py | 54 ++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index b50525e27..f4952db0f 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -2198,3 +2198,57 @@ class TestDomainRequestIncomplete(TestCase): self.domain_request.generic_org_type = None self.domain_request.save() self.assertFalse(self.domain_request._form_complete(request)) + + +class TestPortfolio(TestCase): + def setUp(self): + self.user, _ = User.objects.get_or_create( + username="intern@igorville.com", email="intern@igorville.com", first_name="Lava", last_name="World" + ) + super().setUp() + + def tearDown(self): + super().tearDown() + Portfolio.objects.all().delete() + User.objects.all().delete() + + def test_urbanization_field_resets_when_not_puetro_rico(self): + """The urbanization field should only be populated when the state is puetro rico. + Otherwise, this field should be empty.""" + # Start out as PR, then change the field + portfolio = Portfolio.objects.create( + creator=self.user, + organization_name="Test Portfolio", + state_territory=DomainRequest.StateTerritoryChoices.PUERTO_RICO, + urbanization="test", + ) + + self.assertEqual(portfolio.urbanization, "test") + self.assertEqual(portfolio.state_territory, DomainRequest.StateTerritoryChoices.PUERTO_RICO) + + portfolio.state_territory = DomainRequest.StateTerritoryChoices.ALABAMA + portfolio.save() + + self.assertEqual(portfolio.urbanization, None) + self.assertEqual(portfolio.state_territory, DomainRequest.StateTerritoryChoices.ALABAMA) + + def test_can_add_urbanization_field(self): + """Ensures that you can populate the urbanization field when conditions are right""" + # Create a portfolio that cannot have this field + portfolio = Portfolio.objects.create( + creator=self.user, + organization_name="Test Portfolio", + state_territory=DomainRequest.StateTerritoryChoices.ALABAMA, + urbanization="test", + ) + + # Implicitly check if this gets cleared on create. It should. + self.assertEqual(portfolio.urbanization, None) + self.assertEqual(portfolio.state_territory, DomainRequest.StateTerritoryChoices.ALABAMA) + + portfolio.state_territory = DomainRequest.StateTerritoryChoices.PUERTO_RICO + portfolio.urbanization = "test123" + portfolio.save() + + self.assertEqual(portfolio.urbanization, "test123") + self.assertEqual(portfolio.state_territory, DomainRequest.StateTerritoryChoices.PUERTO_RICO) From 2c4c9ddbce0f59942b1f34074de14163ec1672c1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:18:43 -0600 Subject: [PATCH 14/15] Add url logic to domain requests and domain table --- src/registrar/assets/js/get-gov.js | 25 +++++++++++++++++-- .../includes/domain_requests_table.html | 3 +++ .../templates/includes/domains_table.html | 5 ++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 0712da0f7..7fe0bb9c2 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1183,8 +1183,19 @@ document.addEventListener('DOMContentLoaded', function() { * @param {*} portfolio - the portfolio id */ function loadDomains(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, status = currentStatus, searchTerm = currentSearchTerm, portfolio = portfolioValue) { + // fetch json of page of domais, given params + let baseUrl = document.getElementById("get_domains_json_url"); + if (!baseUrl) { + return; + } + + let baseUrlValue = baseUrl.value; + if (!baseUrlValue) { + return; + } + // fetch json of page of domains, given params - let url = `/get-domains-json/?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}` + let url = `${baseUrlValue}?page=${page}&sort_by=${sortBy}&order=${order}&status=${status}&search_term=${searchTerm}` if (portfolio) url += `&portfolio=${portfolio}` @@ -1524,7 +1535,17 @@ document.addEventListener('DOMContentLoaded', function() { */ function loadDomainRequests(page, sortBy = currentSortBy, order = currentOrder, scroll = scrollToTable, searchTerm = currentSearchTerm) { // fetch json of page of domain requests, given params - fetch(`/get-domain-requests-json/?page=${page}&sort_by=${sortBy}&order=${order}&search_term=${searchTerm}`) + let baseUrl = document.getElementById("get_domain_requests_json_url"); + if (!baseUrl) { + return; + } + + let baseUrlValue = baseUrl.value; + if (!baseUrlValue) { + return; + } + + fetch(`${baseUrlValue}?page=${page}&sort_by=${sortBy}&order=${order}&search_term=${searchTerm}`) .then(response => response.json()) .then(data => { if (data.error) { diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index 30c206741..509ecd525 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -1,5 +1,8 @@ {% load static %} +{% comment %} Stores the json endpoint in a url for easier access {% endcomment %} +{% url 'get_domain_requests_json' as url %} +
{% if not has_domain_requests_portfolio_permission %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 64eddec41..12405a969 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -1,5 +1,10 @@ {% load static %} + + +{% comment %} Stores the json endpoint in a url for easier access {% endcomment %} +{% url 'get_domains_json' as url %} +
{% if not has_domains_portfolio_permission %} From 67cd3684d7b0fb786ad2da6a11901503f93474a1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 14 Aug 2024 12:30:53 -0600 Subject: [PATCH 15/15] input => span --- src/registrar/assets/js/get-gov.js | 4 ++-- src/registrar/templates/includes/domain_requests_table.html | 2 +- src/registrar/templates/includes/domains_table.html | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 7fe0bb9c2..c487e436f 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -1189,7 +1189,7 @@ document.addEventListener('DOMContentLoaded', function() { return; } - let baseUrlValue = baseUrl.value; + let baseUrlValue = baseUrl.innerHTML; if (!baseUrlValue) { return; } @@ -1540,7 +1540,7 @@ document.addEventListener('DOMContentLoaded', function() { return; } - let baseUrlValue = baseUrl.value; + let baseUrlValue = baseUrl.innerHTML; if (!baseUrlValue) { return; } diff --git a/src/registrar/templates/includes/domain_requests_table.html b/src/registrar/templates/includes/domain_requests_table.html index 509ecd525..487c1cee5 100644 --- a/src/registrar/templates/includes/domain_requests_table.html +++ b/src/registrar/templates/includes/domain_requests_table.html @@ -2,7 +2,7 @@ {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} {% url 'get_domain_requests_json' as url %} - +
{% if not has_domain_requests_portfolio_permission %} diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html index 12405a969..1d659650d 100644 --- a/src/registrar/templates/includes/domains_table.html +++ b/src/registrar/templates/includes/domains_table.html @@ -4,7 +4,7 @@ {% comment %} Stores the json endpoint in a url for easier access {% endcomment %} {% url 'get_domains_json' as url %} - +
{% if not has_domains_portfolio_permission %}