From a8fa08acb2afeb0aa719badcd2163899714f1c60 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 4 Feb 2025 06:14:15 -0500
Subject: [PATCH] combined suborg and portfolio permissions

---
 src/registrar/context_processors.py            |  9 ++-------
 src/registrar/models/user.py                   |  9 +--------
 .../models/user_portfolio_permission.py        |  4 ----
 .../models/utility/portfolio_helper.py         |  4 ----
 src/registrar/templates/domain_detail.html     |  8 ++++----
 src/registrar/templates/domain_sidebar.html    |  2 +-
 .../templates/domain_suborganization.html      |  2 +-
 .../templates/includes/domains_table.html      |  2 +-
 src/registrar/tests/test_models.py             | 18 ++----------------
 src/registrar/tests/test_reports.py            |  2 +-
 src/registrar/tests/test_views_domain.py       |  2 +-
 11 files changed, 14 insertions(+), 48 deletions(-)

diff --git a/src/registrar/context_processors.py b/src/registrar/context_processors.py
index b3d9c3727..b22729563 100644
--- a/src/registrar/context_processors.py
+++ b/src/registrar/context_processors.py
@@ -57,11 +57,10 @@ def portfolio_permissions(request):
     """Make portfolio permissions for the request user available in global context"""
     portfolio_context = {
         "has_base_portfolio_permission": False,
+        "has_edit_org_portfolio_permission": False,
         "has_any_domains_portfolio_permission": False,
         "has_any_requests_portfolio_permission": False,
         "has_edit_request_portfolio_permission": False,
-        "has_view_suborganization_portfolio_permission": False,
-        "has_edit_suborganization_portfolio_permission": False,
         "has_view_members_portfolio_permission": False,
         "has_edit_members_portfolio_permission": False,
         "portfolio": None,
@@ -82,15 +81,11 @@ def portfolio_permissions(request):
             }
         )
 
-        # Linting: line too long
-        view_suborg = request.user.has_view_suborganization_portfolio_permission(portfolio)
-        edit_suborg = request.user.has_edit_suborganization_portfolio_permission(portfolio)
         if portfolio:
             return {
                 "has_base_portfolio_permission": request.user.has_base_portfolio_permission(portfolio),
+                "has_edit_org_portfolio_permission": request.user.has_edit_org_portfolio_permission(portfolio),
                 "has_edit_request_portfolio_permission": request.user.has_edit_request_portfolio_permission(portfolio),
-                "has_view_suborganization_portfolio_permission": view_suborg,
-                "has_edit_suborganization_portfolio_permission": edit_suborg,
                 "has_any_domains_portfolio_permission": request.user.has_any_domains_portfolio_permission(portfolio),
                 "has_any_requests_portfolio_permission": request.user.has_any_requests_portfolio_permission(portfolio),
                 "has_view_members_portfolio_permission": request.user.has_view_members_portfolio_permission(portfolio),
diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py
index 1d508f88f..7e0790c5b 100644
--- a/src/registrar/models/user.py
+++ b/src/registrar/models/user.py
@@ -268,13 +268,6 @@ class User(AbstractUser):
     def has_edit_request_portfolio_permission(self, portfolio):
         return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS)
 
-    # Field specific permission checks
-    def has_view_suborganization_portfolio_permission(self, portfolio):
-        return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION)
-
-    def has_edit_suborganization_portfolio_permission(self, portfolio):
-        return self._has_portfolio_permission(portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION)
-
     def is_portfolio_admin(self, portfolio):
         return "Admin" in self.portfolio_role_summary(portfolio)
 
@@ -293,7 +286,7 @@ class User(AbstractUser):
 
         # Define the conditions and their corresponding roles
         conditions_roles = [
-            (self.has_edit_suborganization_portfolio_permission(portfolio), ["Admin"]),
+            (self.has_edit_org_portfolio_permission(portfolio), ["Admin"]),
             (
                 self.has_view_all_domains_portfolio_permission(portfolio)
                 and self.has_any_requests_portfolio_permission(portfolio)
diff --git a/src/registrar/models/user_portfolio_permission.py b/src/registrar/models/user_portfolio_permission.py
index 11d9c56e3..5378dc185 100644
--- a/src/registrar/models/user_portfolio_permission.py
+++ b/src/registrar/models/user_portfolio_permission.py
@@ -27,13 +27,10 @@ class UserPortfolioPermission(TimeStampedModel):
             UserPortfolioPermissionChoices.EDIT_MEMBERS,
             UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
             UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
-            UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION,
-            UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION,
         ],
         # NOTE: Check FORBIDDEN_PORTFOLIO_ROLE_PERMISSIONS before adding roles here.
         UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
             UserPortfolioPermissionChoices.VIEW_PORTFOLIO,
-            UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION,
         ],
     }
 
@@ -43,7 +40,6 @@ class UserPortfolioPermission(TimeStampedModel):
         UserPortfolioRoleChoices.ORGANIZATION_MEMBER: [
             UserPortfolioPermissionChoices.EDIT_PORTFOLIO,
             UserPortfolioPermissionChoices.EDIT_MEMBERS,
-            UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION,
         ],
     }
 
diff --git a/src/registrar/models/utility/portfolio_helper.py b/src/registrar/models/utility/portfolio_helper.py
index 8c42b80c7..2c7b733d5 100644
--- a/src/registrar/models/utility/portfolio_helper.py
+++ b/src/registrar/models/utility/portfolio_helper.py
@@ -41,10 +41,6 @@ class UserPortfolioPermissionChoices(models.TextChoices):
     VIEW_PORTFOLIO = "view_portfolio", "View organization"
     EDIT_PORTFOLIO = "edit_portfolio", "Edit organization"
 
-    # Domain: field specific permissions
-    VIEW_SUBORGANIZATION = "view_suborganization", "View suborganization"
-    EDIT_SUBORGANIZATION = "edit_suborganization", "Edit suborganization"
-
     @classmethod
     def get_user_portfolio_permission_label(cls, user_portfolio_permission):
         return cls(user_portfolio_permission).label if user_portfolio_permission else None
diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html
index 03df2d59c..489d6fdf9 100644
--- a/src/registrar/templates/domain_detail.html
+++ b/src/registrar/templates/domain_detail.html
@@ -103,12 +103,12 @@
     {% endif %}
 
     {% if portfolio %}
-      {% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %}
+      {% if has_any_domains_portfolio_permission and has_edit_org_portfolio_permission %}
         {% url 'domain-suborganization' pk=domain.id as url %}
-        {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_suborganization_portfolio_permission %}
-    {% elif has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %}
+        {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_edit_org_portfolio_permission %}
+    {% elif has_any_domains_portfolio_permission and has_base_portfolio_permission %}
         {% url 'domain-suborganization' pk=domain.id as url %}
-        {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_view_suborganization_portfolio_permission view_button=True %}
+        {% include "includes/summary_item.html" with title='Suborganization' value=domain.domain_info.sub_organization edit_link=url editable=is_editable|and:has_base_portfolio_permission view_button=True %}
         {% endif %}
     {% else %}
       {% url 'domain-org-name-address' pk=domain.id as url %}
diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html
index ca3802720..a87a611cd 100644
--- a/src/registrar/templates/domain_sidebar.html
+++ b/src/registrar/templates/domain_sidebar.html
@@ -61,7 +61,7 @@
       
       {% if portfolio %}
           {% comment %} Only show this menu option if the user has the perms to do so {% endcomment %}
-          {% if has_any_domains_portfolio_permission and has_view_suborganization_portfolio_permission %}
+          {% if has_any_domains_portfolio_permission and has_base_portfolio_permission %}
             {% with url_name="domain-suborganization" %}
               {% include "includes/domain_sidenav_item.html" with item_text="Suborganization" %}
             {% endwith %}
diff --git a/src/registrar/templates/domain_suborganization.html b/src/registrar/templates/domain_suborganization.html
index e050690c8..89ce4e79d 100644
--- a/src/registrar/templates/domain_suborganization.html
+++ b/src/registrar/templates/domain_suborganization.html
@@ -39,7 +39,7 @@
         please contact <a href="mailto:help@get.gov" class="usa-link">help@get.gov</a>.
     </p>
 
-    {% if has_any_domains_portfolio_permission and has_edit_suborganization_portfolio_permission %}
+    {% if has_any_domains_portfolio_permission and has_edit_org_portfolio_permission %}
         <form class="usa-form usa-form--large" method="post" novalidate id="form-container">
             {% csrf_token %}
             {% input_with_errors form.sub_organization %}
diff --git a/src/registrar/templates/includes/domains_table.html b/src/registrar/templates/includes/domains_table.html
index de3d15eb0..9a49e46f9 100644
--- a/src/registrar/templates/includes/domains_table.html
+++ b/src/registrar/templates/includes/domains_table.html
@@ -208,7 +208,7 @@
           <th data-sortable="name" scope="col" role="columnheader">Domain name</th>
           <th data-sortable="expiration_date" scope="col" role="columnheader">Expires</th>
           <th data-sortable="state_display" scope="col" role="columnheader">Status</th>
-          {% if portfolio and has_view_suborganization_portfolio_permission %}
+          {% if portfolio and has_base_portfolio_permission %}
             <th data-sortable="domain_info__sub_organization" scope="col" role="columnheader">Suborganization</th>
           {% endif %}
           <th 
diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py
index 157420be7..4cd353d36 100644
--- a/src/registrar/tests/test_models.py
+++ b/src/registrar/tests/test_models.py
@@ -1190,8 +1190,8 @@ class TestUser(TestCase):
         User.objects.all().delete()
         UserDomainRole.objects.all().delete()
 
-    @patch.object(User, "has_edit_suborganization_portfolio_permission", return_value=True)
-    def test_portfolio_role_summary_admin(self, mock_edit_suborganization):
+    @patch.object(User, "has_edit_org_portfolio_permission", return_value=True)
+    def test_portfolio_role_summary_admin(self, mock_edit_org):
         # Test if the user is recognized as an Admin
         self.assertEqual(self.user.portfolio_role_summary(self.portfolio), ["Admin"])
 
@@ -1305,20 +1305,6 @@ class TestUser(TestCase):
         self.assertTrue(self.user.has_edit_request_portfolio_permission(self.portfolio))
         mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_REQUESTS)
 
-    @patch("registrar.models.User._has_portfolio_permission")
-    def test_has_view_suborganization_portfolio_permission(self, mock_has_permission):
-        mock_has_permission.return_value = True
-
-        self.assertTrue(self.user.has_view_suborganization_portfolio_permission(self.portfolio))
-        mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.VIEW_SUBORGANIZATION)
-
-    @patch("registrar.models.User._has_portfolio_permission")
-    def test_has_edit_suborganization_portfolio_permission(self, mock_has_permission):
-        mock_has_permission.return_value = True
-
-        self.assertTrue(self.user.has_edit_suborganization_portfolio_permission(self.portfolio))
-        mock_has_permission.assert_called_once_with(self.portfolio, UserPortfolioPermissionChoices.EDIT_SUBORGANIZATION)
-
     @less_console_noise_decorator
     def test_check_transition_domains_without_domains_on_login(self):
         """A user's on_each_login callback does not check transition domains.
diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index 4ba5b5bc9..bab4f327b 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -725,7 +725,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip()
             self.assertEqual(csv_content, expected_content)
 
-    # @less_console_noise_decorator
+    @less_console_noise_decorator
     def test_domain_request_data_full(self):
         """Tests the full domain request report."""
         # Remove "Submitted at" because we can't guess this immutable, dynamically generated test data
diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py
index 02b61fda9..dc5bff27a 100644
--- a/src/registrar/tests/test_views_domain.py
+++ b/src/registrar/tests/test_views_domain.py
@@ -2181,7 +2181,7 @@ class TestDomainSuborganization(TestDomainOverview):
         self.domain_information.refresh_from_db()
 
         # Add portfolio perms to the user object
-        portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(
+        UserPortfolioPermission.objects.get_or_create(
             user=self.user, portfolio=portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
         )