From 06aacd91ce464fd808d792a04f285b34e2954a05 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 29 Aug 2024 10:58:19 -0400
Subject: [PATCH] minor improvements

---
 src/registrar/assets/js/get-gov-admin.js | 51 +++++++++---------------
 src/registrar/tests/test_api.py          | 11 ++++-
 2 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/src/registrar/assets/js/get-gov-admin.js b/src/registrar/assets/js/get-gov-admin.js
index 25dd91e0aa..24f020b754 100644
--- a/src/registrar/assets/js/get-gov-admin.js
+++ b/src/registrar/assets/js/get-gov-admin.js
@@ -908,10 +908,6 @@ function initializeWidgetOnList(list, parentId) {
             return;
         }
 
-        // Hide the contactList initially. 
-        // If we can update the contact information, it'll be shown again.
-        hideElement(contactList.parentElement);
-
         // Determine if any changes are necessary to the display of portfolio type or federal type
         // based on changes to the Federal Agency
         let federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value;
@@ -925,14 +921,15 @@ function initializeWidgetOnList(list, parentId) {
                 console.error("Error in AJAX call: " + data.error);
                 return;
             }
-
-            let federal_type = data.federal_type;
-            let portfolio_type = data.portfolio_type;
-            updateFederalType(data.federal_type);
-            updatePortfolioType(data.portfolio_type);
+            updateReadOnly(data.federal_type, '.field-federal_type');
+            updateReadOnly(data.portfolio_type, '.field-portfolio_type');
         })
         .catch(error => console.error("Error fetching federal and portfolio types: ", error));
 
+        // Hide the contactList initially. 
+        // If we can update the contact information, it'll be shown again.
+        hideElement(contactList.parentElement);
+        
         let seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value;
         fetch(`${seniorOfficialApi}?agency_name=${selectedText}`)
         .then(response => {
@@ -988,33 +985,21 @@ function initializeWidgetOnList(list, parentId) {
     }
 
     /**
-     * Dynamically update the portfolio type text in the dom to portfolioType
-     */
-    function updatePortfolioType(portfolioType) {
-        // Find the div with class 'field-portfolio_type'
-        const portfolioTypeDiv = document.querySelector('.field-portfolio_type');
-        if (portfolioTypeDiv) {
-            // Find the nested div with class 'readonly' inside 'field-portfolio_type'
-            const readonlyDiv = portfolioTypeDiv.querySelector('.readonly');
-            if (readonlyDiv) {
-                // Update the text content of the readonly div
-                readonlyDiv.textContent = portfolioType !== null ? portfolioType : '-';
-            }
-        }
-    }
-
-    /**
-     * Dynamically update the federal type text in the dom to federalType
+     * Utility that selects a div from the DOM using selectorString,
+     * and updates a div within that div which has class of 'readonly'
+     * so that the text of the div is updated to updateText
+     * @param {*} updateText 
+     * @param {*} selectorString 
      */
-    function updateFederalType(federalType) {
-        // Find the div with class 'field-federal_type'
-        const federalTypeDiv = document.querySelector('.field-federal_type');
-        if (federalTypeDiv) {
-            // Find the nested div with class 'readonly' inside 'field-federal_type'
-            const readonlyDiv = federalTypeDiv.querySelector('.readonly');
+    function updateReadOnly(updateText, selectorString) {
+        // find the div by selectorString
+        const selectedDiv = document.querySelector(selectorString);
+        if (selectedDiv) {
+            // find the nested div with class 'readonly' inside the selectorString div
+            const readonlyDiv = selectedDiv.querySelector('.readonly');
             if (readonlyDiv) {
                 // Update the text content of the readonly div
-                readonlyDiv.textContent = federalType !== null ? federalType : '-';
+                readonlyDiv.textContent = updateText !== null ? updateText : '-';
             }
         }
     }
diff --git a/src/registrar/tests/test_api.py b/src/registrar/tests/test_api.py
index 00bc9a1a24..2597e65c2e 100644
--- a/src/registrar/tests/test_api.py
+++ b/src/registrar/tests/test_api.py
@@ -5,6 +5,7 @@
 from registrar.tests.common import create_superuser, create_user
 
 from api.tests.common import less_console_noise_decorator
+from registrar.utility.constants import BranchChoices
 
 
 class GetSeniorOfficialJsonTest(TestCase):
@@ -82,7 +83,7 @@ def setUp(self):
         self.superuser = create_superuser()
         self.analyst_user = create_user()
 
-        self.agency = FederalAgency.objects.create(agency="Test Agency", federal_type="judicial")
+        self.agency = FederalAgency.objects.create(agency="Test Agency", federal_type=BranchChoices.JUDICIAL)
 
         self.api_url = reverse("get-federal-and-portfolio-types-from-federal-agency-json")
 
@@ -100,3 +101,11 @@ def test_get_federal_and_portfolio_types_json_authenticated_superuser(self):
         data = response.json()
         self.assertEqual(data["federal_type"], "Judicial")
         self.assertEqual(data["portfolio_type"], "Federal - Judicial")
+
+    @less_console_noise_decorator
+    def test_get_federal_and_portfolio_types_json_authenticated_regularuser(self):
+        """Test that a regular 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", "organization_type": "federal"})
+        self.assertEqual(response.status_code, 302)