From 4dd16ec37058486a6b9f9f729eb77c8f815f75a0 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 3 Mar 2025 16:53:28 -0500
Subject: [PATCH 01/47] add omb analyst group, add omb analyst permission,
 refine is_staff decorator as appropriate

---
 src/registrar/decorators.py                   |  6 ++
 .../migrations/0141_create_groups_v18.py      | 38 +++++++
 src/registrar/models/user.py                  |  1 +
 src/registrar/models/user_group.py            | 98 +++++++++++++++++++
 src/registrar/views/report_views.py           | 20 ++--
 src/registrar/views/transfer_user.py          |  4 +-
 src/registrar/views/utility/api_views.py      | 50 ++--------
 7 files changed, 162 insertions(+), 55 deletions(-)
 create mode 100644 src/registrar/migrations/0141_create_groups_v18.py

diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py
index 7cb2792f4..8188b152e 100644
--- a/src/registrar/decorators.py
+++ b/src/registrar/decorators.py
@@ -6,6 +6,9 @@ from registrar.models import Domain, DomainInformation, DomainInvitation, Domain
 # Constants for clarity
 ALL = "all"
 IS_STAFF = "is_staff"
+IS_CISA_ANALYST = "is_cisa_analyst"
+IS_OMB_ANALYST = "is_omb_analyst"
+IS_FULL_ACCESS = "is_full_access"
 IS_DOMAIN_MANAGER = "is_domain_manager"
 IS_DOMAIN_REQUEST_CREATOR = "is_domain_request_creator"
 IS_STAFF_MANAGING_DOMAIN = "is_staff_managing_domain"
@@ -101,6 +104,9 @@ def _user_has_permission(user, request, rules, **kwargs):
     # Define permission checks
     permission_checks = [
         (IS_STAFF, lambda: user.is_staff),
+        (IS_CISA_ANALYST, lambda: user.has_perm("registrar.analyst_access_permission")),
+        (IS_OMB_ANALYST, lambda: user.has_perm("registrar.omb_analyst_access_permission")),
+        (IS_FULL_ACCESS, lambda: user.has_perm("registrar.full_access_permission")),
         (IS_DOMAIN_MANAGER, lambda: _is_domain_manager(user, **kwargs)),
         (IS_STAFF_MANAGING_DOMAIN, lambda: _is_staff_managing_domain(request, **kwargs)),
         (IS_PORTFOLIO_MEMBER, lambda: user.is_org_user(request)),
diff --git a/src/registrar/migrations/0141_create_groups_v18.py b/src/registrar/migrations/0141_create_groups_v18.py
new file mode 100644
index 000000000..c416f0f1e
--- /dev/null
+++ b/src/registrar/migrations/0141_create_groups_v18.py
@@ -0,0 +1,38 @@
+# This migration creates the create_full_access_group and create_cisa_analyst_group groups
+# It is dependent on 0079 (which populates federal agencies)
+# If permissions on the groups need changing, edit CISA_ANALYST_GROUP_PERMISSIONS
+# in the user_group model then:
+# [NOT RECOMMENDED]
+# step 1: docker-compose exec app ./manage.py migrate --fake registrar 0035_contenttypes_permissions
+# step 2: docker-compose exec app ./manage.py migrate registrar 0036_create_groups
+# step 3: fake run the latest migration in the migrations list
+# [RECOMMENDED]
+# Alternatively:
+# step 1: duplicate the migration that loads data
+# step 2: docker-compose exec app ./manage.py migrate
+
+from django.db import migrations
+from registrar.models import UserGroup
+from typing import Any
+
+
+# For linting: RunPython expects a function reference,
+# so let's give it one
+def create_groups(apps, schema_editor) -> Any:
+    UserGroup.create_cisa_analyst_group(apps, schema_editor)
+    UserGroup.create_omb_analyst_group(apps, schema_editor)
+    UserGroup.create_full_access_group(apps, schema_editor)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("registrar", "0140_alter_portfolioinvitation_additional_permissions_and_more"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            create_groups,
+            reverse_code=migrations.RunPython.noop,
+            atomic=True,
+        ),
+    ]
diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py
index d5476ab9a..7ee4fefdf 100644
--- a/src/registrar/models/user.py
+++ b/src/registrar/models/user.py
@@ -40,6 +40,7 @@ class User(AbstractUser):
 
         permissions = [
             ("analyst_access_permission", "Analyst Access Permission"),
+            ("omb_analyst_access_permission", "OMB Analyst Access Permission"),
             ("full_access_permission", "Full Access Permission"),
         ]
 
diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index 4770f34bc..b3bddee66 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -141,6 +141,104 @@ class UserGroup(Group):
         except Exception as e:
             logger.error(f"Error creating analyst permissions group: {e}")
 
+    def create_omb_analyst_group(apps, schema_editor):
+        """This method gets run from a data migration."""
+
+        # Hard to pass self to these methods as the calls from migrations
+        # are only expecting apps and schema_editor, so we'll just define
+        # apps, schema_editor in the local scope instead
+        OMB_ANALYST_GROUP_PERMISSIONS = [
+            {
+                "app_label": "registrar",
+                "model": "domainrequest",
+                "permissions": ["change_domainrequest"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "domain",
+                "permissions": ["view_domain"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "user",
+                "permissions": ["omb_analyst_access_permission"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "domaininvitation",
+                "permissions": ["view_domaininvitation"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "federalagency",
+                "permissions": ["change_federalagency", "delete_federalagency"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "portfolio",
+                "permissions": ["change_portfolio", "delete_portfolio"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "suborganization",
+                "permissions": ["change_suborganization", "delete_suborganization"],
+            },
+            {
+                "app_label": "registrar",
+                "model": "seniorofficial",
+                "permissions": ["change_seniorofficial", "delete_seniorofficial"],
+            },
+        ]
+
+        # Avoid error: You can't execute queries until the end
+        # of the 'atomic' block.
+        # From django docs:
+        # https://docs.djangoproject.com/en/4.2/topics/migrations/#data-migrations
+        # We can’t import the Person model directly as it may be a newer
+        # version than this migration expects. We use the historical version.
+        ContentType = apps.get_model("contenttypes", "ContentType")
+        Permission = apps.get_model("auth", "Permission")
+        UserGroup = apps.get_model("registrar", "UserGroup")
+
+        logger.info("Going to create the OMB Analyst Group")
+        try:
+            omb_analysts_group, _ = UserGroup.objects.get_or_create(
+                name="omb_analysts_group",
+            )
+
+            omb_analysts_group.permissions.clear()
+
+            for permission in OMB_ANALYST_GROUP_PERMISSIONS:
+                app_label = permission["app_label"]
+                model_name = permission["model"]
+                permissions = permission["permissions"]
+
+                # Retrieve the content type for the app and model
+                content_type = ContentType.objects.get(app_label=app_label, model=model_name)
+
+                # Retrieve the permissions based on their codenames
+                permissions = Permission.objects.filter(content_type=content_type, codename__in=permissions)
+
+                # Assign the permissions to the group
+                omb_analysts_group.permissions.add(*permissions)
+
+                # Convert the permissions QuerySet to a list of codenames
+                permission_list = list(permissions.values_list("codename", flat=True))
+
+                logger.debug(
+                    app_label
+                    + " | "
+                    + model_name
+                    + " | "
+                    + ", ".join(permission_list)
+                    + " added to group "
+                    + omb_analysts_group.name
+                )
+
+                logger.debug("OMB Analyst permissions added to group " + omb_analysts_group.name)
+        except Exception as e:
+            logger.error(f"Error creating analyst permissions group: {e}")
+
     def create_full_access_group(apps, schema_editor):
         """This method gets run from a data migration."""
 
diff --git a/src/registrar/views/report_views.py b/src/registrar/views/report_views.py
index c07dcfc1b..7f1e63e32 100644
--- a/src/registrar/views/report_views.py
+++ b/src/registrar/views/report_views.py
@@ -6,7 +6,7 @@ from django.shortcuts import render
 from django.contrib import admin
 from django.db.models import Avg, F
 
-from registrar.decorators import ALL, HAS_PORTFOLIO_MEMBERS_VIEW, IS_STAFF, grant_access
+from registrar.decorators import ALL, HAS_PORTFOLIO_MEMBERS_VIEW, IS_CISA_ANALYST, IS_FULL_ACCESS, grant_access
 from .. import models
 import datetime
 from django.utils import timezone
@@ -16,7 +16,7 @@ import logging
 logger = logging.getLogger(__name__)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class AnalyticsView(View):
     def get(self, request):
         thirty_days_ago = datetime.datetime.today() - datetime.timedelta(days=30)
@@ -176,7 +176,7 @@ class AnalyticsView(View):
         return render(request, "admin/analytics.html", context)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataType(View):
     def get(self, request, *args, **kwargs):
         # match the CSV example with all the fields
@@ -227,7 +227,7 @@ class ExportMembersPortfolio(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataFull(View):
     def get(self, request, *args, **kwargs):
         # Smaller export based on 1
@@ -237,7 +237,7 @@ class ExportDataFull(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataFederal(View):
     def get(self, request, *args, **kwargs):
         # Federal only
@@ -247,7 +247,7 @@ class ExportDataFederal(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDomainRequestDataFull(View):
     """Generates a downloaded report containing all Domain Requests (except started)"""
 
@@ -259,7 +259,7 @@ class ExportDomainRequestDataFull(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataDomainsGrowth(View):
     def get(self, request, *args, **kwargs):
         start_date = request.GET.get("start_date", "")
@@ -272,7 +272,7 @@ class ExportDataDomainsGrowth(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataRequestsGrowth(View):
     def get(self, request, *args, **kwargs):
         start_date = request.GET.get("start_date", "")
@@ -285,7 +285,7 @@ class ExportDataRequestsGrowth(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataManagedDomains(View):
     def get(self, request, *args, **kwargs):
         start_date = request.GET.get("start_date", "")
@@ -297,7 +297,7 @@ class ExportDataManagedDomains(View):
         return response
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class ExportDataUnmanagedDomains(View):
     def get(self, request, *args, **kwargs):
         start_date = request.GET.get("start_date", "")
diff --git a/src/registrar/views/transfer_user.py b/src/registrar/views/transfer_user.py
index 62cd0a9d2..ee8ebad35 100644
--- a/src/registrar/views/transfer_user.py
+++ b/src/registrar/views/transfer_user.py
@@ -4,7 +4,7 @@ from django.db.models import ForeignKey, OneToOneField, ManyToManyField, ManyToO
 
 from django.shortcuts import render, get_object_or_404, redirect
 from django.views import View
-from registrar.decorators import IS_STAFF, grant_access
+from registrar.decorators import IS_CISA_ANALYST, IS_FULL_ACCESS, grant_access
 from registrar.models.domain import Domain
 from registrar.models.domain_request import DomainRequest
 from registrar.models.user import User
@@ -19,7 +19,7 @@ from registrar.utility.db_helpers import ignore_unique_violation
 logger = logging.getLogger(__name__)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_FULL_ACCESS)
 class TransferUserView(View):
     """Transfer user methods that set up the transfer_user template and handle the forms on it."""
 
diff --git a/src/registrar/views/utility/api_views.py b/src/registrar/views/utility/api_views.py
index 6d0a2b5ec..ea794e185 100644
--- a/src/registrar/views/utility/api_views.py
+++ b/src/registrar/views/utility/api_views.py
@@ -1,7 +1,7 @@
 import logging
 from django.http import JsonResponse
 from django.forms.models import model_to_dict
-from registrar.decorators import IS_STAFF, grant_access
+from registrar.decorators import IS_CISA_ANALYST, IS_FULL_ACCESS, IS_OMB_ANALYST, grant_access
 from registrar.models import FederalAgency, SeniorOfficial, DomainRequest
 from registrar.utility.admin_helpers import get_action_needed_reason_default_email, get_rejection_reason_default_email
 from registrar.models.portfolio import Portfolio
@@ -10,16 +10,10 @@ from registrar.utility.constants import BranchChoices
 logger = logging.getLogger(__name__)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 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)
-
     agency_name = request.GET.get("agency_name")
     agency = FederalAgency.objects.filter(agency=agency_name).first()
     senior_official = SeniorOfficial.objects.filter(federal_agency=agency).first()
@@ -37,16 +31,10 @@ def get_senior_official_from_federal_agency_json(request):
         return JsonResponse({"error": "Senior Official not found"}, status=404)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 def get_portfolio_json(request):
     """Returns portfolio 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)
-
     portfolio_id = request.GET.get("id")
     try:
         portfolio = Portfolio.objects.get(id=portfolio_id)
@@ -93,16 +81,10 @@ def get_portfolio_json(request):
     return JsonResponse(portfolio_dict)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 def get_suborganization_list_json(request):
     """Returns suborganization list information for a portfolio 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)
-
     portfolio_id = request.GET.get("portfolio_id")
     try:
         portfolio = Portfolio.objects.get(id=portfolio_id)
@@ -115,17 +97,11 @@ def get_suborganization_list_json(request):
     return JsonResponse({"results": results, "pagination": {"more": False}})
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 def get_federal_and_portfolio_types_from_federal_agency_json(request):
     """Returns specific portfolio information as a JSON. Request must have
     both agency_name and organization_type."""
 
-    # 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)
-
     federal_type = None
     portfolio_type = None
 
@@ -143,16 +119,10 @@ def get_federal_and_portfolio_types_from_federal_agency_json(request):
     return JsonResponse(response_data)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 def get_action_needed_email_for_user_json(request):
     """Returns a default action needed email for a given user"""
 
-    # 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)
-
     reason = request.GET.get("reason")
     domain_request_id = request.GET.get("domain_request_id")
     if not reason:
@@ -167,16 +137,10 @@ def get_action_needed_email_for_user_json(request):
     return JsonResponse({"email": email}, status=200)
 
 
-@grant_access(IS_STAFF)
+@grant_access(IS_CISA_ANALYST, IS_OMB_ANALYST, IS_FULL_ACCESS)
 def get_rejection_email_for_user_json(request):
     """Returns a default rejection email for a given user"""
 
-    # 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)
-
     reason = request.GET.get("reason")
     domain_request_id = request.GET.get("domain_request_id")
     if not reason:

From 562ff46faa8ed44ee12b97507f1d9851a8579722 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 3 Mar 2025 17:05:57 -0500
Subject: [PATCH 02/47] refine permissions on analytics

---
 src/registrar/templates/admin/app_list.html | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/registrar/templates/admin/app_list.html b/src/registrar/templates/admin/app_list.html
index ecce12a3e..6f30e3031 100644
--- a/src/registrar/templates/admin/app_list.html
+++ b/src/registrar/templates/admin/app_list.html
@@ -63,6 +63,7 @@
       </table>
     </div>
   {% endfor %}
+  {% if perms.registrar.analyst_access_permission or perms.full_access_permission %}
   <div class="module">
     <table class="width-full">
       <caption class="text-bold">Analytics</caption>
@@ -78,6 +79,7 @@
       </tbody>
     </table>
   </div>
+  {% endif %}
 {% else %}
   <p>{% translate 'You don’t have permission to view or edit anything.' %}</p>
 {% endif %}

From a93dbdc5cc1375a7837794a5d46f9619c37385da Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 3 Mar 2025 17:20:31 -0500
Subject: [PATCH 03/47] import and export buttons and functions limited by
 permission

---
 src/registrar/admin.py | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 4b05bbb6d..7808a7cc7 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -75,6 +75,15 @@ from django.utils.translation import gettext_lazy as _
 logger = logging.getLogger(__name__)
 
 
+class ImportExportRegistrarModelAdmin(ImportExportModelAdmin):
+
+    def has_import_permission(self, request):
+        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm("registrar.full_access_permission")
+
+    def has_export_permission(self, request):
+        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm("registrar.full_access_permission")
+
+
 class FsmModelResource(resources.ModelResource):
     """ModelResource is extended to support importing of tables which
     have FSMFields.  ModelResource is extended with the following changes
@@ -751,7 +760,7 @@ class ListHeaderAdmin(AuditedAdmin, OrderableFieldsMixin):
         return filters
 
 
-class MyUserAdmin(BaseUserAdmin, ImportExportModelAdmin):
+class MyUserAdmin(BaseUserAdmin, ImportExportRegistrarModelAdmin):
     """Custom user admin class to use our inlines."""
 
     resource_classes = [UserResource]
@@ -1044,7 +1053,7 @@ class HostResource(resources.ModelResource):
         model = models.Host
 
 
-class MyHostAdmin(AuditedAdmin, ImportExportModelAdmin):
+class MyHostAdmin(AuditedAdmin, ImportExportRegistrarModelAdmin):
     """Custom host admin class to use our inlines."""
 
     resource_classes = [HostResource]
@@ -1070,7 +1079,7 @@ class HostIpResource(resources.ModelResource):
         model = models.HostIP
 
 
-class HostIpAdmin(AuditedAdmin, ImportExportModelAdmin):
+class HostIpAdmin(AuditedAdmin, ImportExportRegistrarModelAdmin):
     """Custom host ip admin class"""
 
     resource_classes = [HostIpResource]
@@ -1093,7 +1102,7 @@ class ContactResource(resources.ModelResource):
         model = models.Contact
 
 
-class ContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class ContactAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom contact admin class to add search."""
 
     resource_classes = [ContactResource]
@@ -1244,7 +1253,7 @@ class WebsiteResource(resources.ModelResource):
         model = models.Website
 
 
-class WebsiteAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class WebsiteAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom website admin class."""
 
     resource_classes = [WebsiteResource]
@@ -1344,7 +1353,7 @@ class UserPortfolioPermissionAdmin(ListHeaderAdmin):
             obj.delete()  # Calls the overridden delete method on each instance
 
 
-class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class UserDomainRoleAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom user domain role admin class."""
 
     resource_classes = [UserDomainRoleResource]
@@ -1760,7 +1769,7 @@ class DomainInformationResource(resources.ModelResource):
         model = models.DomainInformation
 
 
-class DomainInformationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class DomainInformationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Customize domain information admin class."""
 
     class GenericOrgFilter(admin.SimpleListFilter):
@@ -2098,7 +2107,7 @@ class DomainRequestResource(FsmModelResource):
         model = models.DomainRequest
 
 
-class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom domain requests admin class."""
 
     resource_classes = [DomainRequestResource]
@@ -3309,7 +3318,7 @@ class DomainResource(FsmModelResource):
         model = models.Domain
 
 
-class DomainAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom domain admin class to add extra buttons."""
 
     resource_classes = [DomainResource]
@@ -3902,7 +3911,7 @@ class DraftDomainResource(resources.ModelResource):
         model = models.DraftDomain
 
 
-class DraftDomainAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class DraftDomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom draft domain admin class."""
 
     resource_classes = [DraftDomainResource]
@@ -4022,7 +4031,7 @@ class PublicContactResource(resources.ModelResource):
         self.after_save_instance(instance, using_transactions, dry_run)
 
 
-class PublicContactAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class PublicContactAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     """Custom PublicContact admin class."""
 
     resource_classes = [PublicContactResource]
@@ -4358,7 +4367,7 @@ class FederalAgencyResource(resources.ModelResource):
         model = models.FederalAgency
 
 
-class FederalAgencyAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     list_display = ["agency"]
     search_fields = ["agency"]
     search_help_text = "Search by federal agency."
@@ -4415,11 +4424,11 @@ class WaffleFlagAdmin(FlagAdmin):
         return super().changelist_view(request, extra_context=extra_context)
 
 
-class DomainGroupAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class DomainGroupAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     list_display = ["name", "portfolio"]
 
 
-class SuborganizationAdmin(ListHeaderAdmin, ImportExportModelAdmin):
+class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
     list_display = ["name", "portfolio"]
     autocomplete_fields = [

From 2bd188b267a185ccaa715d75dd4e0108918113e4 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 4 Mar 2025 06:38:34 -0500
Subject: [PATCH 04/47] filtered admin views based on specific permission
 groups

---
 src/registrar/admin.py | 193 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 193 insertions(+)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 7808a7cc7..d58ded2f9 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1244,6 +1244,32 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
     # in autocomplete_fields for Senior Official
     ordering = ["first_name", "last_name"]
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            converted_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(federal_agency__isnull=False),
+                    then=F("federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=Value(""),
+            ),
+        )
+    
+    def get_queryset(self, request):
+        """Restrict queryset based on user permissions."""
+        qs = super().get_queryset(request)
+
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                converted_federal_type=BranchChoices.EXECUTIVE,
+            )
+
+        return qs  # Return full queryset if the user doesn't have the restriction
+    
 
 class WebsiteResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -1536,6 +1562,39 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
     # Override for the delete confirmation page on the domain table (bulk delete action)
     delete_selected_confirmation_template = "django/admin/domain_invitation_delete_selected_confirmation.html"
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            converted_generic_org_type=Case(
+                # When portfolio is present, use its value instead
+                When(domain__domain_info__portfolio__isnull=False, then=F("domain__domain_info__portfolio__organization_type")),
+                # Otherwise, return the natively assigned value
+                default=F("domain__domain_info__generic_org_type"),
+            ),
+            converted_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(domain__domain_info__portfolio__isnull=False) & Q(domain__domain_info__portfolio__federal_agency__isnull=False),
+                    then=F("domain__domain_info__portfolio__federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=F("domain__domain_info__federal_agency__federal_type"),
+            ),
+        )
+    
+    def get_queryset(self, request):
+        """Restrict queryset based on user permissions."""
+        qs = super().get_queryset(request)
+
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                converted_generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                converted_federal_type=BranchChoices.EXECUTIVE,
+            )
+
+        return qs  # Return full queryset if the user doesn't have the restriction
+    
     # Select domain invitations to change -> Domain invitations
     def changelist_view(self, request, extra_context=None):
         if extra_context is None:
@@ -2098,6 +2157,38 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         use_sort = db_field.name != "senior_official"
         return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            conv_generic_org_type=Case(
+                # When portfolio is present, use its value instead
+                When(portfolio__isnull=False, then=F("portfolio__organization_type")),
+                # Otherwise, return the natively assigned value
+                default=F("generic_org_type"),
+            ),
+            conv_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
+                    then=F("portfolio__federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=F("federal_agency__federal_type"),
+            ),
+        )
+
+    def get_queryset(self, request):
+        """Custom get_queryset to filter by portfolio if portfolio is in the
+        request params."""
+        qs = super().get_queryset(request)
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                conv_generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                conv_federal_type=BranchChoices.EXECUTIVE,
+            )
+        return qs
+
 
 class DomainRequestResource(FsmModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -3050,6 +3141,25 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         use_sort = db_field.name != "senior_official"
         return super().formfield_for_foreignkey(db_field, request, use_admin_sort_fields=use_sort, **kwargs)
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            conv_generic_org_type=Case(
+                # When portfolio is present, use its value instead
+                When(portfolio__isnull=False, then=F("portfolio__organization_type")),
+                # Otherwise, return the natively assigned value
+                default=F("generic_org_type"),
+            ),
+            conv_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
+                    then=F("portfolio__federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=F("federal_agency__federal_type"),
+            ),
+        )
+
     def get_queryset(self, request):
         """Custom get_queryset to filter by portfolio if portfolio is in the
         request params."""
@@ -3059,6 +3169,13 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         if portfolio_id:
             # Further filter the queryset by the portfolio
             qs = qs.filter(portfolio=portfolio_id)
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                conv_generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                conv_federal_type=BranchChoices.EXECUTIVE,
+            )
         return qs
 
     def get_search_results(self, request, queryset, search_term):
@@ -3900,6 +4017,12 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         if portfolio_id:
             # Further filter the queryset by the portfolio
             qs = qs.filter(domain_info__portfolio=portfolio_id)
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            return qs.filter(
+                converted_generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                converted_federal_type=BranchChoices.EXECUTIVE,
+            )
         return qs
 
 
@@ -4314,6 +4437,34 @@ class PortfolioAdmin(ListHeaderAdmin):
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
         return readonly_fields
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            converted_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(federal_agency__isnull=False),
+                    then=F("federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=Value(""),
+            ),
+        )
+    
+    def get_queryset(self, request):
+        """Restrict queryset based on user permissions."""
+        qs = super().get_queryset(request)
+
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+                converted_federal_type=BranchChoices.EXECUTIVE,
+            )
+
+        return qs  # Return full queryset if the user doesn't have the restriction
+    
+
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
         Add the summary for the portfolio members field (list of members that link to change_forms)."""
@@ -4374,6 +4525,17 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     ordering = ["agency"]
     resource_classes = [FederalAgencyResource]
 
+    def get_queryset(self, request):
+        """Restrict queryset based on user permissions."""
+        qs = super().get_queryset(request)
+
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            return qs.filter(
+                federal_type=BranchChoices.EXECUTIVE,
+            )
+
+        return qs  # Return full queryset if the user doesn't have the restriction
 
 class UserGroupAdmin(AuditedAdmin):
     """Overwrite the generated UserGroup admin class"""
@@ -4456,6 +4618,37 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         extra_context = {"domain_requests": domain_requests, "domains": domains}
         return super().change_view(request, object_id, form_url, extra_context)
 
+    def get_annotated_queryset(self, queryset):
+        return queryset.annotate(
+            converted_federal_type=Case(
+                # When portfolio is present, use its value instead
+                When(
+                    Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
+                    then=F("portfolio__federal_agency__federal_type"),
+                ),
+                # Otherwise, return the natively assigned value
+                default=Value(""),
+            ),
+        )
+
+    def get_queryset(self, request):
+        """Custom get_queryset to filter by portfolio if portfolio is in the
+        request params."""
+        qs = super().get_queryset(request)
+        # Check if a 'portfolio' parameter is passed in the request
+        portfolio_id = request.GET.get("portfolio")
+        if portfolio_id:
+            # Further filter the queryset by the portfolio
+            qs = qs.filter(portfolio=portfolio_id)
+        # Check if user is in OMB analysts group
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            annotated_qs = self.get_annotated_queryset(qs)
+            return annotated_qs.filter(
+                portfolio__organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+                converted_federal_type=BranchChoices.EXECUTIVE,
+            )
+        return qs
+
 
 class AllowedEmailAdmin(ListHeaderAdmin):
     class Meta:

From 16bcae0dc281cad5ab03c917d01e57150933eb53 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 4 Mar 2025 20:48:22 -0500
Subject: [PATCH 05/47] added admin object and group specific permissions for
 view, add, change and or delete

---
 src/registrar/admin.py | 152 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 151 insertions(+), 1 deletion(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index d58ded2f9..387712bbb 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1270,6 +1270,33 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
 
         return qs  # Return full queryset if the user doesn't have the restriction
     
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
+        return super().has_view_permission(request, obj)
+    
+    def has_change_permission(self, request, obj=None):
+        """Restrict update permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """Restrict delete permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
+        return super().has_delete_permisssion(request, obj)
+
 
 class WebsiteResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -1595,6 +1622,16 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
 
         return qs  # Return full queryset if the user doesn't have the restriction
     
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+        return super().has_view_permission(request, obj)
+    
     # Select domain invitations to change -> Domain invitations
     def changelist_view(self, request, extra_context=None):
         if extra_context is None:
@@ -3177,7 +3214,27 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 conv_federal_type=BranchChoices.EXECUTIVE,
             )
         return qs
-
+    
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+        return super().has_view_permission(request, obj)
+    
+    def has_change_permission(self, request, obj=None):
+        """Restrict update permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+        return super().has_change_permission(request, obj)
+    
     def get_search_results(self, request, queryset, search_term):
         # Call the parent's method to apply default search logic
         base_queryset, use_distinct = super().get_search_results(request, queryset, search_term)
@@ -4025,6 +4082,16 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             )
         return qs
 
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.domain_info.converted_federal_type == BranchChoices.EXECUTIVE        
+        return super().has_view_permission(request, obj)
+    
 
 class DraftDomainResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -4464,6 +4531,32 @@ class PortfolioAdmin(ListHeaderAdmin):
 
         return qs  # Return full queryset if the user doesn't have the restriction
     
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_type == BranchChoices.EXECUTIVE
+        return super().has_view_permission(request, obj)
+    
+    def has_change_permission(self, request, obj=None):
+        """Restrict update permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_type == BranchChoices.EXECUTIVE        
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """Restrict delete permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_type == BranchChoices.EXECUTIVE
+        return super().has_delete_permisssion(request, obj)
 
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
@@ -4537,6 +4630,36 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
         return qs  # Return full queryset if the user doesn't have the restriction
 
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+        return super().has_view_permission(request, obj)
+    
+    def has_change_permission(self, request, obj=None):
+        """Restrict update permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
+                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """Restrict delete permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.federal_type == BranchChoices.EXECUTIVE
+        return super().has_delete_permisssion(request, obj)
+    
+
 class UserGroupAdmin(AuditedAdmin):
     """Overwrite the generated UserGroup admin class"""
 
@@ -4648,6 +4771,33 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 converted_federal_type=BranchChoices.EXECUTIVE,
             )
         return qs
+    
+    def has_view_permission(self, request, obj=None):
+        """Restrict view permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
+        return super().has_view_permission(request, obj)
+    
+    def has_change_permission(self, request, obj=None):
+        """Restrict update permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE     
+        return super().has_change_permission(request, obj)
+
+    def has_delete_permission(self, request, obj=None):
+        """Restrict delete permissions based on group membership and model attributes."""
+        if request.user.has_perm("registrar.full_access_permission"):
+            return True
+        if obj:
+            if request.user.groups.filter(name="omb_analysts_group").exists():
+                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
+        return super().has_delete_permisssion(request, obj)
 
 
 class AllowedEmailAdmin(ListHeaderAdmin):

From 996d8eaccfe12e9f8ca74868880251e8a8f4756d Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Wed, 5 Mar 2025 18:30:17 -0500
Subject: [PATCH 06/47] changes to domain request admin form

---
 src/registrar/admin.py                        | 70 +++++++++++++++++--
 .../admin/includes/contact_detail_list.html   |  6 +-
 .../admin/includes/detail_table_fieldset.html | 16 ++++-
 3 files changed, 84 insertions(+), 8 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 387712bbb..e16463fb8 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -323,9 +323,10 @@ class DomainRequestAdminForm(forms.ModelForm):
             # only set the available transitions if the user is not restricted
             # from editing the domain request; otherwise, the form will be
             # readonly and the status field will not have a widget
-            if not domain_request.creator.is_restricted():
+            if not domain_request.creator.is_restricted() and "status" in self.fields:
                 self.fields["status"].widget.choices = available_transitions
 
+
     def get_custom_field_transitions(self, instance, field):
         """Custom implementation of get_available_FIELD_transitions
         in the FSM. Allows us to still display fields filtered out by a condition."""
@@ -1295,7 +1296,7 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permisssion(request, obj)
+        return super().has_delete_permission(request, obj)
 
 
 class WebsiteResource(resources.ModelResource):
@@ -2749,6 +2750,53 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         "cisa_representative_email",
     ]
 
+    # Read only that we'll leverage for OMB Analysts
+    omb_analyst_readonly_fields = [
+        "federal_agency",
+        "creator",
+        "about_your_organization",
+        "requested_domain",
+        "approved_domain",
+        "alternative_domains",
+        "purpose",
+        "no_other_contacts_rationale",
+        "anything_else",
+        "is_policy_acknowledged",
+        "cisa_representative_first_name",
+        "cisa_representative_last_name",
+        "cisa_representative_email",
+        "status",
+        "investigator",
+        "notes",
+        "senior_official",
+        "organization_type",
+        "organization_name",
+        "state_territory",
+        "address_line1",
+        "address_line2",
+        "city",
+        "zipcode",
+        "urbanization",
+        "portfolio_organization_type",
+        "portfolio_federal_type",
+        "portfolio_organization_name",
+        "portfolio_federal_agency",
+        "portfolio_state_territory",
+        "portfolio_address_line1",
+        "portfolio_address_line2",
+        "portfolio_city",
+        "portfolio_zipcode",
+        "portfolio_urbanization",
+        "is_election_board",
+        "organization_type",
+        "federal_type",
+        "federal_agency",
+        "tribe_name",
+        "federally_recognized_tribe",
+        "state_recognized_tribe",
+        "about_your_organization",
+    ]
+
     autocomplete_fields = [
         "approved_domain",
         "requested_domain",
@@ -2990,6 +3038,10 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
         if request.user.has_perm("registrar.full_access_permission"):
             return readonly_fields
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
         # Return restrictive Read-only fields for analysts and
         # users who might not belong to groups
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
@@ -3254,6 +3306,14 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
         return combined_queryset, use_distinct
 
+    def get_form(self, request, obj=None, **kwargs):
+        """Pass the 'is_omb_analyst' attribute to the form."""
+        form = super().get_form(request, obj, **kwargs)
+
+        # Store attribute in the form for template access
+        form.show_contact_as_plain_text = request.user.groups.filter(name="omb_analysts_group").exists()
+
+        return form
 
 class TransitionDomainAdmin(ListHeaderAdmin):
     """Custom transition domain admin class."""
@@ -4556,7 +4616,7 @@ class PortfolioAdmin(ListHeaderAdmin):
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permisssion(request, obj)
+        return super().has_delete_permission(request, obj)
 
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
@@ -4657,7 +4717,7 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permisssion(request, obj)
+        return super().has_delete_permission(request, obj)
     
 
 class UserGroupAdmin(AuditedAdmin):
@@ -4797,7 +4857,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permisssion(request, obj)
+        return super().has_delete_permission(request, obj)
 
 
 class AllowedEmailAdmin(ListHeaderAdmin):
diff --git a/src/registrar/templates/django/admin/includes/contact_detail_list.html b/src/registrar/templates/django/admin/includes/contact_detail_list.html
index 0baabac17..b3cdeb875 100644
--- a/src/registrar/templates/django/admin/includes/contact_detail_list.html
+++ b/src/registrar/templates/django/admin/includes/contact_detail_list.html
@@ -6,7 +6,11 @@
 
     {% if show_formatted_name %}
         {% if user.get_formatted_name %}
-            <a class="contact_info_name" href="{% url 'admin:registrar_contact_change' user.id %}">{{ user.get_formatted_name }}</a>
+            {% if adminform.form.show_contact_as_plain_text %}
+                {{ user.get_formatted_name }}
+            {% else %}
+                <a class="contact_info_name" href="{% url 'admin:registrar_contact_change' user.id %}">{{ user.get_formatted_name }}</a>
+            {% endif %}
         {% else %}
             None
         {% endif %}
diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html
index a074e8a7c..c800a7c84 100644
--- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html
@@ -69,7 +69,11 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
     {% elif field.field.name == "portfolio_senior_official" %}
         <div class="readonly">
             {% if original_object.portfolio.senior_official %}
-                <a href="{% url 'admin:registrar_seniorofficial_change' original_object.portfolio.senior_official.id %}">{{ field.contents }}</a>
+                {% if adminform.form.show_contact_as_plain_text %}
+                    {{ field.contents|striptags }}
+                {% else %}
+                    <a href="{% url 'admin:registrar_seniorofficial_change' original_object.portfolio.senior_official.id %}">{{ field.contents }}</a>
+                {% endif %}
             {% else %}
                 No senior official found.<br>
             {% endif %}
@@ -78,7 +82,11 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
         {% if all_contacts.count > 2 %}
             <div class="readonly">
                 {% for contact in all_contacts %}
-                    <a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a>{% if not forloop.last %}, {% endif %}
+                    {% if adminform.form.show_contact_as_plain_text %}
+                        {{ contact.get_formatted_name }}{% if not forloop.last %}, {% endif %}
+                    {% else %}
+                        <a href="{% url 'admin:registrar_contact_change' contact.id %}">{{ contact.get_formatted_name }}</a>{% if not forloop.last %}, {% endif %}
+                    {% endif %}
                 {% endfor %}
             </div>
         {% else %}
@@ -153,6 +161,10 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
                 <p>No additional members found.</p>
             {% endif %}
         </div>
+    {% elif field.field.name == "creator" and adminform.form.show_contact_as_plain_text %}
+        <div class="readonly">{{ field.contents|striptags }}</div>
+    {% elif field.field.name == "senior_official" and adminform.form.show_contact_as_plain_text %}
+        <div class="readonly">{{ field.contents|striptags }}</div>
     {% else %}
         <div class="readonly">{{ field.contents }}</div>
     {% endif %}

From 70970fbcc56266ed97e64e1df619b1a61a9a4c20 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 08:55:31 -0500
Subject: [PATCH 07/47]  portfolio admin updates

---
 src/registrar/admin.py                        | 130 +++++++++++++++---
 src/registrar/models/user_group.py            |   2 +-
 .../django/admin/domain_change_form.html      |   6 +-
 .../portfolio/portfolio_admins_table.html     |   6 +-
 .../portfolio/portfolio_fieldset.html         |   3 +
 5 files changed, 124 insertions(+), 23 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index e16463fb8..f556db16d 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -326,7 +326,6 @@ class DomainRequestAdminForm(forms.ModelForm):
             if not domain_request.creator.is_restricted() and "status" in self.fields:
                 self.fields["status"].widget.choices = available_transitions
 
-
     def get_custom_field_transitions(self, instance, field):
         """Custom implementation of get_available_FIELD_transitions
         in the FSM. Allows us to still display fields filtered out by a condition."""
@@ -3345,6 +3344,16 @@ class DomainInformationInline(admin.StackedInline):
     template = "django/admin/includes/domain_info_inline_stacked.html"
     model = models.DomainInformation
 
+    def __init__(self, *args, **kwargs):
+        """Initialize the admin class and define a default value for is_omb_analyst."""
+        super().__init__(*args, **kwargs)
+        self.is_omb_analyst = False  # Default value in case it's accessed before being set
+
+    def get_queryset(self, request):
+        """Ensure self.is_omb_analyst is set early."""
+        self.is_omb_analyst = request.user.groups.filter(name="omb_analysts_group").exists()
+        return super().get_queryset(request)
+    
     # Define methods to display fields from the related portfolio
     def portfolio_senior_official(self, obj) -> Optional[SeniorOfficial]:
         return obj.portfolio.senior_official if obj.portfolio and obj.portfolio.senior_official else None
@@ -3432,12 +3441,16 @@ class DomainInformationInline(admin.StackedInline):
         if not domain_managers:
             return "No domain managers found."
 
-        domain_manager_details = "<table><thead><tr><th>UID</th><th>Name</th><th>Email</th></tr></thead><tbody>"
+        domain_manager_details = "<table><thead><tr>"
+        if not self.is_omb_analyst:
+            domain_manager_details += "<th>UID</th>"
+        domain_manager_details += "<th>Name</th><th>Email</th></tr></thead><tbody>"
         for domain_manager in domain_managers:
             full_name = domain_manager.get_formatted_name()
             change_url = reverse("admin:registrar_user_change", args=[domain_manager.pk])
             domain_manager_details += "<tr>"
-            domain_manager_details += f'<td><a href="{change_url}">{escape(domain_manager.username)}</a>'
+            if not self.is_omb_analyst:
+                domain_manager_details += f'<td><a href="{change_url}">{escape(domain_manager.username)}</a>'
             domain_manager_details += f"<td>{escape(full_name)}</td>"
             domain_manager_details += f"<td>{escape(domain_manager.email)}</td>"
             domain_manager_details += "</tr>"
@@ -3469,7 +3482,8 @@ class DomainInformationInline(admin.StackedInline):
 
         superuser_perm = request.user.has_perm("registrar.full_access_permission")
         analyst_perm = request.user.has_perm("registrar.analyst_access_permission")
-        if analyst_perm and not superuser_perm:
+        omb_analyst_perm = request.user.groups.filter(name="omb_analysts_group").exists()
+        if (analyst_perm or omb_analyst_perm) and not superuser_perm:
             return True
         return super().has_change_permission(request, obj)
 
@@ -3542,6 +3556,17 @@ class DomainInformationInline(admin.StackedInline):
                 modified_fieldsets.append(fieldsets_to_move)
 
         return modified_fieldsets
+    
+    def get_form(self, request, obj=None, **kwargs):
+        """Pass the 'is_omb_analyst' attribute to the form."""
+        form = super().get_form(request, obj, **kwargs)
+
+        # Store attribute in the form for template access
+        self.is_omb_analyst = request.user.groups.filter(name="omb_analysts_group").exists()
+        form.show_contact_as_plain_text = self.is_omb_analyst
+        form.is_omb_analyst = self.is_omb_analyst
+
+        return form
 
 
 class DomainResource(FsmModelResource):
@@ -4152,7 +4177,17 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                     obj.domain_info.converted_federal_type == BranchChoices.EXECUTIVE        
         return super().has_view_permission(request, obj)
     
+    def get_form(self, request, obj=None, **kwargs):
+        """Pass the 'is_omb_analyst' attribute to the form."""
+        form = super().get_form(request, obj, **kwargs)
 
+        # Store attribute in the form for template access
+        is_omb_analyst = request.user.groups.filter(name="omb_analysts_group").exists()
+        form.show_contact_as_plain_text = is_omb_analyst
+        form.is_omb_analyst = is_omb_analyst
+
+        return form
+    
 class DraftDomainResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
     import/export file"""
@@ -4336,6 +4371,11 @@ class PortfolioAdmin(ListHeaderAdmin):
 
     _meta = Meta()
 
+    def __init__(self, *args, **kwargs):
+        """Initialize the admin class and define a default value for is_omb_analyst."""
+        super().__init__(*args, **kwargs)
+        self.is_omb_analyst = False  # Default value in case it's accessed before being set
+
     change_form_template = "django/admin/portfolio_change_form.html"
     fieldsets = [
         # created_on is the created_at field
@@ -4417,6 +4457,19 @@ class PortfolioAdmin(ListHeaderAdmin):
     # rather than strip it out of our logic.
     analyst_readonly_fields = []  # type: ignore
 
+    omb_analyst_readonly_fields = [
+        "notes",
+        "organization_type",
+        "organization_name",
+        "federal_agency",
+        "state_territory",
+        "address_line1",
+        "address_line2",
+        "city",
+        "zipcode",
+        "urbanization",
+    ]
+
     def get_admin_users(self, obj):
         # Filter UserPortfolioPermission objects related to the portfolio
         admin_permissions = self.get_user_portfolio_permission_admins(obj)
@@ -4502,6 +4555,8 @@ class PortfolioAdmin(ListHeaderAdmin):
         """Returns the number of administrators for this portfolio"""
         admin_count = len(self.get_user_portfolio_permission_admins(obj))
         if admin_count > 0:
+            if self.is_omb_analyst:
+                return format_html(f"{admin_count} administrators")
             url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}"
             # Create a clickable link with the count
             return format_html(f'<a href="{url}">{admin_count} administrators</a>')
@@ -4513,6 +4568,8 @@ class PortfolioAdmin(ListHeaderAdmin):
         """Returns the number of members for this portfolio"""
         member_count = len(self.get_user_portfolio_permission_non_admins(obj))
         if member_count > 0:
+            if self.is_omb_analyst:
+                return format_html(f"{member_count} members")
             url = reverse("admin:registrar_userportfoliopermission_changelist") + f"?portfolio={obj.id}"
             # Create a clickable link with the count
             return format_html(f'<a href="{url}">{member_count} members</a>')
@@ -4558,7 +4615,10 @@ class PortfolioAdmin(ListHeaderAdmin):
 
         if request.user.has_perm("registrar.full_access_permission"):
             return readonly_fields
-
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
         # Return restrictive Read-only fields for analysts and
         # users who might not belong to groups
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
@@ -4583,6 +4643,7 @@ class PortfolioAdmin(ListHeaderAdmin):
 
         # Check if user is in OMB analysts group
         if request.user.groups.filter(name="omb_analysts_group").exists():
+            self.is_omb_analyst = True
             annotated_qs = self.get_annotated_queryset(qs)
             return annotated_qs.filter(
                 organization_type=DomainRequest.OrganizationChoices.FEDERAL,
@@ -4609,15 +4670,6 @@ class PortfolioAdmin(ListHeaderAdmin):
                 return obj.federal_type == BranchChoices.EXECUTIVE        
         return super().has_change_permission(request, obj)
 
-    def has_delete_permission(self, request, obj=None):
-        """Restrict delete permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permission(request, obj)
-
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
         Add the summary for the portfolio members field (list of members that link to change_forms)."""
@@ -4662,6 +4714,17 @@ class PortfolioAdmin(ListHeaderAdmin):
 
         super().save_model(request, obj, form, change)
 
+    def get_form(self, request, obj=None, **kwargs):
+        """Pass the 'is_omb_analyst' attribute to the form."""
+        form = super().get_form(request, obj, **kwargs)
+
+        # Store attribute in the form for template access
+        self.is_omb_analyst = request.user.groups.filter(name="omb_analysts_group").exists()
+        form.show_contact_as_plain_text = self.is_omb_analyst
+        form.is_omb_analyst = self.is_omb_analyst
+
+        return form
+
 
 class FederalAgencyResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -4678,6 +4741,20 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     ordering = ["agency"]
     resource_classes = [FederalAgencyResource]
 
+    # Readonly fields for analysts and superusers
+    readonly_fields = []
+
+    # Read only that we'll leverage for CISA Analysts
+    analyst_readonly_fields = []
+
+    # Read only that we'll leverage for OMB Analysts
+    omb_analyst_readonly_fields = [
+        "agency",
+        "federal_type",
+        "acronym",
+        "is_fceb",
+    ]
+
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -4696,8 +4773,7 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+                return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
     
     def has_change_permission(self, request, obj=None):
@@ -4706,8 +4782,7 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+                return obj.federal_type == BranchChoices.EXECUTIVE        
         return super().has_change_permission(request, obj)
 
     def has_delete_permission(self, request, obj=None):
@@ -4718,7 +4793,24 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_delete_permission(request, obj)
-    
+
+    def get_readonly_fields(self, request, obj=None):
+        """Set the read-only state on form elements.
+        We have 2 conditions that determine which fields are read-only:
+        admin user permissions and the domain request creator's status, so
+        we'll use the baseline readonly_fields and extend it as needed.
+        """
+        readonly_fields = list(self.readonly_fields)
+        if request.user.has_perm("registrar.full_access_permission"):
+            return readonly_fields
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields  
 
 class UserGroupAdmin(AuditedAdmin):
     """Overwrite the generated UserGroup admin class"""
diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index b3bddee66..031fe8e06 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -176,7 +176,7 @@ class UserGroup(Group):
             {
                 "app_label": "registrar",
                 "model": "portfolio",
-                "permissions": ["change_portfolio", "delete_portfolio"],
+                "permissions": ["change_portfolio"],
             },
             {
                 "app_label": "registrar",
diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html
index 7aa0034b9..9f34feae6 100644
--- a/src/registrar/templates/django/admin/domain_change_form.html
+++ b/src/registrar/templates/django/admin/domain_change_form.html
@@ -11,13 +11,15 @@
 {% block field_sets %}
     <div class="display-flex flex-row flex-justify submit-row">
         <div class="flex-align-self-start button-list-mobile">
+            {% if not adminform.form.is_omb_analyst %}
             <input id="manageDomainSubmitButton" type="submit" value="Manage domain" name="_edit_domain">
             {# Dja has margin styles defined on inputs as is. Lets work with it, rather than fight it. #}
             <span class="mini-spacer"></span>
             <input type="submit" value="Get registry status" name="_get_status">
+            {% endif %}
         </div>
         <div class="desktop:flex-align-self-end">
-            {% if original.state != original.State.DELETED %}
+            {% if original.state != original.State.DELETED and not adminform.form.is_omb_analyst %}
             <a class="text-middle" href="#toggle-extend-expiration-alert" aria-controls="toggle-extend-expiration-alert" data-open-modal>
                 Extend expiration date
             </a>
@@ -33,7 +35,7 @@
             {% if original.state == original.State.READY or original.state == original.State.ON_HOLD %}
             <span class="margin-left-05 margin-right-05 text-middle"> | </span>
             {% endif %}
-            {% if original.state != original.State.DELETED %}
+            {% if original.state != original.State.DELETED and not adminform.form.is_omb_analyst %}
             <a class="text-middle" href="#toggle-remove-from-registry" aria-controls="toggle-remove-from-registry" data-open-modal>
                 Remove from registry
             </a>
diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html
index 7add74323..574c05738 100644
--- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html
+++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_admins_table.html
@@ -16,7 +16,11 @@
         {% for admin in admins %}
         {% url 'admin:registrar_userportfoliopermission_change' admin.pk as url %}
         <tr>
-            <td><a href={{url}}>{{ admin.user.get_formatted_name}}</a></td>
+            {% if adminform.form.is_omb_analyst %}
+                <td>{{ admin.user.get_formatted_name }}</td>
+            {% else %}
+                <td><a href={{url}}>{{ admin.user.get_formatted_name}}</a></td>
+            {% endif %}
             <td>{{ admin.user.title }}</td>
             <td>
                 {% if admin.user.email %}
diff --git a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html
index 87b56cb60..54ac502d1 100644
--- a/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html
+++ b/src/registrar/templates/django/admin/includes/portfolio/portfolio_fieldset.html
@@ -30,6 +30,9 @@
                 <a href={{ url }}>No senior official found. Create one now.</a>
             </div>
         {% endif %}
+
+    {% elif field.field.name == "creator" and adminform.form.show_contact_as_plain_text %}
+        <div class="readonly">{{ field.contents|striptags }}</div>
     {% else %}
         <div class="readonly">{{ field.contents }}</div>
     {% endif %}

From 76bb219a2a8325d7d8c798c338828a0834fde7ea Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 13:55:00 -0500
Subject: [PATCH 08/47] suborgs and senior officials

---
 src/registrar/admin.py             | 84 +++++++++++++++++++++++-------
 src/registrar/models/user_group.py |  4 +-
 2 files changed, 68 insertions(+), 20 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index f556db16d..1f5dae5f1 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1257,6 +1257,40 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
             ),
         )
     
+    readonly_fields = []
+
+    # Even though this is empty, I will leave it as a stub for easy changes in the future
+    # rather than strip it out of our logic.
+    analyst_readonly_fields = []  # type: ignore
+
+    omb_analyst_readonly_fields = [
+        "first_name",
+        "last_name",
+        "title",
+        "phone",
+        "email",
+        "federal_agency",
+    ]
+
+    def get_readonly_fields(self, request, obj=None):
+        """Set the read-only state on form elements.
+        We have conditions that determine which fields are read-only:
+        admin user permissions and analyst (cisa or omb) status, so
+        we'll use the baseline readonly_fields and extend it as needed.
+        """
+        readonly_fields = list(self.readonly_fields)
+
+        if request.user.has_perm("registrar.full_access_permission"):
+            return readonly_fields
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields
+    
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -1288,15 +1322,6 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
                 return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
         return super().has_change_permission(request, obj)
 
-    def has_delete_permission(self, request, obj=None):
-        """Restrict delete permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permission(request, obj)
-
 
 class WebsiteResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -4876,6 +4901,38 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
     change_form_template = "django/admin/suborg_change_form.html"
 
+    readonly_fields = []
+
+    # Even though this is empty, I will leave it as a stub for easy changes in the future
+    # rather than strip it out of our logic.
+    analyst_readonly_fields = []  # type: ignore
+
+    omb_analyst_readonly_fields = [
+        "name",
+        "portfolio",
+        "city",
+        "state_territory",
+    ]
+
+    def get_readonly_fields(self, request, obj=None):
+        """Set the read-only state on form elements.
+        We have conditions that determine which fields are read-only:
+        admin user permissions and analyst (cisa or omb) status, so
+        we'll use the baseline readonly_fields and extend it as needed.
+        """
+        readonly_fields = list(self.readonly_fields)
+
+        if request.user.has_perm("registrar.full_access_permission"):
+            return readonly_fields
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
+        # Return restrictive Read-only fields for analysts and
+        # users who might not belong to groups
+        readonly_fields.extend([field for field in self.analyst_readonly_fields])
+        return readonly_fields
+
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add suborg's related domains and requests to context"""
         obj = self.get_object(request, object_id)
@@ -4942,15 +4999,6 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE     
         return super().has_change_permission(request, obj)
 
-    def has_delete_permission(self, request, obj=None):
-        """Restrict delete permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permission(request, obj)
-
 
 class AllowedEmailAdmin(ListHeaderAdmin):
     class Meta:
diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index 031fe8e06..781dbb64c 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -181,12 +181,12 @@ class UserGroup(Group):
             {
                 "app_label": "registrar",
                 "model": "suborganization",
-                "permissions": ["change_suborganization", "delete_suborganization"],
+                "permissions": ["change_suborganization"],
             },
             {
                 "app_label": "registrar",
                 "model": "seniorofficial",
-                "permissions": ["change_seniorofficial", "delete_seniorofficial"],
+                "permissions": ["change_seniorofficial"],
             },
         ]
 

From 7d2a37970fc9e67bc518dc8153ffd9a79cbcfafd Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 14:00:21 -0500
Subject: [PATCH 09/47] lint

---
 src/registrar/admin.py | 93 +++++++++++++++++++++++++-----------------
 1 file changed, 56 insertions(+), 37 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 1f5dae5f1..303d82f05 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -78,10 +78,14 @@ logger = logging.getLogger(__name__)
 class ImportExportRegistrarModelAdmin(ImportExportModelAdmin):
 
     def has_import_permission(self, request):
-        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm("registrar.full_access_permission")
+        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm(
+            "registrar.full_access_permission"
+        )
 
     def has_export_permission(self, request):
-        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm("registrar.full_access_permission")
+        return request.user.has_perm("registrar.analyst_access_permission") or request.user.has_perm(
+            "registrar.full_access_permission"
+        )
 
 
 class FsmModelResource(resources.ModelResource):
@@ -1256,7 +1260,7 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
                 default=Value(""),
             ),
         )
-    
+
     readonly_fields = []
 
     # Even though this is empty, I will leave it as a stub for easy changes in the future
@@ -1290,7 +1294,7 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
         # users who might not belong to groups
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
         return readonly_fields
-    
+
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -1303,7 +1307,7 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
             )
 
         return qs  # Return full queryset if the user doesn't have the restriction
-    
+
     def has_view_permission(self, request, obj=None):
         """Restrict view permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
@@ -1312,7 +1316,7 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
-    
+
     def has_change_permission(self, request, obj=None):
         """Restrict update permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
@@ -1618,21 +1622,25 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
         return queryset.annotate(
             converted_generic_org_type=Case(
                 # When portfolio is present, use its value instead
-                When(domain__domain_info__portfolio__isnull=False, then=F("domain__domain_info__portfolio__organization_type")),
+                When(
+                    domain__domain_info__portfolio__isnull=False,
+                    then=F("domain__domain_info__portfolio__organization_type"),
+                ),
                 # Otherwise, return the natively assigned value
                 default=F("domain__domain_info__generic_org_type"),
             ),
             converted_federal_type=Case(
                 # When portfolio is present, use its value instead
                 When(
-                    Q(domain__domain_info__portfolio__isnull=False) & Q(domain__domain_info__portfolio__federal_agency__isnull=False),
+                    Q(domain__domain_info__portfolio__isnull=False)
+                    & Q(domain__domain_info__portfolio__federal_agency__isnull=False),
                     then=F("domain__domain_info__portfolio__federal_agency__federal_type"),
                 ),
                 # Otherwise, return the natively assigned value
                 default=F("domain__domain_info__federal_agency__federal_type"),
             ),
         )
-    
+
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -1646,17 +1654,19 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
             )
 
         return qs  # Return full queryset if the user doesn't have the restriction
-    
+
     def has_view_permission(self, request, obj=None):
         """Restrict view permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+                return (
+                    obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL
+                    and obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+                )
         return super().has_view_permission(request, obj)
-    
+
     # Select domain invitations to change -> Domain invitations
     def changelist_view(self, request, extra_context=None):
         if extra_context is None:
@@ -3290,27 +3300,31 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 conv_federal_type=BranchChoices.EXECUTIVE,
             )
         return qs
-    
+
     def has_view_permission(self, request, obj=None):
         """Restrict view permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+                return (
+                    obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL
+                    and obj.converted_federal_type == BranchChoices.EXECUTIVE
+                )
         return super().has_view_permission(request, obj)
-    
+
     def has_change_permission(self, request, obj=None):
         """Restrict update permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.converted_federal_type == BranchChoices.EXECUTIVE        
+                return (
+                    obj.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL
+                    and obj.converted_federal_type == BranchChoices.EXECUTIVE
+                )
         return super().has_change_permission(request, obj)
-    
+
     def get_search_results(self, request, queryset, search_term):
         # Call the parent's method to apply default search logic
         base_queryset, use_distinct = super().get_search_results(request, queryset, search_term)
@@ -3339,6 +3353,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
         return form
 
+
 class TransitionDomainAdmin(ListHeaderAdmin):
     """Custom transition domain admin class."""
 
@@ -3378,7 +3393,7 @@ class DomainInformationInline(admin.StackedInline):
         """Ensure self.is_omb_analyst is set early."""
         self.is_omb_analyst = request.user.groups.filter(name="omb_analysts_group").exists()
         return super().get_queryset(request)
-    
+
     # Define methods to display fields from the related portfolio
     def portfolio_senior_official(self, obj) -> Optional[SeniorOfficial]:
         return obj.portfolio.senior_official if obj.portfolio and obj.portfolio.senior_official else None
@@ -3581,7 +3596,7 @@ class DomainInformationInline(admin.StackedInline):
                 modified_fieldsets.append(fieldsets_to_move)
 
         return modified_fieldsets
-    
+
     def get_form(self, request, obj=None, **kwargs):
         """Pass the 'is_omb_analyst' attribute to the form."""
         form = super().get_form(request, obj, **kwargs)
@@ -4198,10 +4213,12 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL and \
-                    obj.domain_info.converted_federal_type == BranchChoices.EXECUTIVE        
+                return (
+                    obj.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL
+                    and obj.domain_info.converted_federal_type == BranchChoices.EXECUTIVE
+                )
         return super().has_view_permission(request, obj)
-    
+
     def get_form(self, request, obj=None, **kwargs):
         """Pass the 'is_omb_analyst' attribute to the form."""
         form = super().get_form(request, obj, **kwargs)
@@ -4212,7 +4229,8 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         form.is_omb_analyst = is_omb_analyst
 
         return form
-    
+
+
 class DraftDomainResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
     import/export file"""
@@ -4661,7 +4679,7 @@ class PortfolioAdmin(ListHeaderAdmin):
                 default=Value(""),
             ),
         )
-    
+
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -4676,7 +4694,7 @@ class PortfolioAdmin(ListHeaderAdmin):
             )
 
         return qs  # Return full queryset if the user doesn't have the restriction
-    
+
     def has_view_permission(self, request, obj=None):
         """Restrict view permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
@@ -4685,14 +4703,14 @@ class PortfolioAdmin(ListHeaderAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
-    
+
     def has_change_permission(self, request, obj=None):
         """Restrict update permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE        
+                return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_change_permission(request, obj)
 
     def change_view(self, request, object_id, form_url="", extra_context=None):
@@ -4770,7 +4788,7 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     readonly_fields = []
 
     # Read only that we'll leverage for CISA Analysts
-    analyst_readonly_fields = []
+    analyst_readonly_fields = []  # type: ignore
 
     # Read only that we'll leverage for OMB Analysts
     omb_analyst_readonly_fields = [
@@ -4800,14 +4818,14 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
-    
+
     def has_change_permission(self, request, obj=None):
         """Restrict update permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE        
+                return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_change_permission(request, obj)
 
     def has_delete_permission(self, request, obj=None):
@@ -4835,7 +4853,8 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         # Return restrictive Read-only fields for analysts and
         # users who might not belong to groups
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
-        return readonly_fields  
+        return readonly_fields
+
 
 class UserGroupAdmin(AuditedAdmin):
     """Overwrite the generated UserGroup admin class"""
@@ -4980,7 +4999,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 converted_federal_type=BranchChoices.EXECUTIVE,
             )
         return qs
-    
+
     def has_view_permission(self, request, obj=None):
         """Restrict view permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
@@ -4989,14 +5008,14 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
-    
+
     def has_change_permission(self, request, obj=None):
         """Restrict update permissions based on group membership and model attributes."""
         if request.user.has_perm("registrar.full_access_permission"):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE     
+                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
         return super().has_change_permission(request, obj)
 
 

From 97be3dc56d8b1cbae2d6f7de094ec5ccf2a329e5 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 15:00:41 -0500
Subject: [PATCH 10/47] additional migration

---
 .../migrations/0143_alter_user_options.py     | 23 +++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100644 src/registrar/migrations/0143_alter_user_options.py

diff --git a/src/registrar/migrations/0143_alter_user_options.py b/src/registrar/migrations/0143_alter_user_options.py
new file mode 100644
index 000000000..58e5cf3d5
--- /dev/null
+++ b/src/registrar/migrations/0143_alter_user_options.py
@@ -0,0 +1,23 @@
+# Generated by Django 4.2.17 on 2025-03-06 20:00
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("registrar", "0142_create_groups_v18"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="user",
+            options={
+                "permissions": [
+                    ("analyst_access_permission", "Analyst Access Permission"),
+                    ("omb_analyst_access_permission", "OMB Analyst Access Permission"),
+                    ("full_access_permission", "Full Access Permission"),
+                ]
+            },
+        ),
+    ]

From fe1e64828b7fefc2146254af3b588addaa4ca597 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 19:18:22 -0500
Subject: [PATCH 11/47] fix placing holds on domains

---
 src/registrar/admin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 71884b4a7..e3ef6f0ad 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -4327,7 +4327,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         # but cannot access this page when it is a request of type POST.
         if request.user.has_perm("registrar.full_access_permission") or request.user.has_perm(
             "registrar.analyst_access_permission"
-        ):
+        ) or request.user.has_perm("registrar.omb_analyst_access_permission"):
             return True
         return super().has_change_permission(request, obj)
 

From 343d8dc962f4a371018b5652ff50dc5fa5cd9018 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 22:00:37 -0500
Subject: [PATCH 12/47] fixed javascript and other issues on domain request
 admin

---
 src/registrar/admin.py                        |   4 +
 .../js/getgov-admin/domain-request-form.js    | 233 +++++++++++-------
 .../helpers-portfolio-dynamic-fields.js       |   4 +-
 3 files changed, 149 insertions(+), 92 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index e3ef6f0ad..d9b9509e1 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -2976,6 +2976,10 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         "federally_recognized_tribe",
         "state_recognized_tribe",
         "about_your_organization",
+        "rejection_reason",
+        "rejection_reason_email",
+        "action_needed_reason",
+        "action_needed_reason_email",
     ]
 
     autocomplete_fields = [
diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
index db6467875..8823cb46c 100644
--- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
@@ -106,7 +106,9 @@ export function initApprovedDomain() {
         }
 
         const statusToCheck = "approved";
+        const readonlyStatusToCheck = "Approved";
         const statusSelect = document.getElementById("id_status");
+        const statusField = document.querySelector("field-status");
         const sessionVariableName = "showApprovedDomain";
         let approvedDomainFormGroup = document.querySelector(".field-approved_domain");
 
@@ -120,18 +122,30 @@ export function initApprovedDomain() {
 
         // Handle showing/hiding the related fields on page load.
         function initializeFormGroups() {
-            let isStatus = statusSelect.value == statusToCheck;
+            let isStatus = false;
+            if (statusSelect) {
+                isStatus = statusSelect.value == statusToCheck;
+            } else {
+                // statusSelect does not exist, indicating readonly
+                if (statusField) {
+                    let readonlyDiv = statusField.querySelector("div.readonly");
+                    let readonlyStatusText = readonlyDiv.textContent.trim();
+                    isStatus = readonlyStatusText == readonlyStatusToCheck;
+                }
+            }
 
             // Initial handling of these groups.
             updateFormGroupVisibility(isStatus);
 
-            // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage
-            statusSelect.addEventListener('change', () => {
-                // Show the approved if the status is what we expect.
-                isStatus = statusSelect.value == statusToCheck;
-                updateFormGroupVisibility(isStatus);
-                addOrRemoveSessionBoolean(sessionVariableName, isStatus);
-            });
+            if (statusSelect) {
+                // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage
+                statusSelect.addEventListener('change', () => {
+                    // Show the approved if the status is what we expect.
+                    isStatus = statusSelect.value == statusToCheck;
+                    updateFormGroupVisibility(isStatus);
+                    addOrRemoveSessionBoolean(sessionVariableName, isStatus);
+                });
+            }
             
             // Listen to Back/Forward button navigation and handle approvedDomainFormGroup display based on session storage
             // When you navigate using forward/back after changing status but not saving, when you land back on the DA page the
@@ -322,6 +336,7 @@ class CustomizableEmailBase {
      * @property {HTMLElement} modalConfirm - The confirm button in the modal.
      * @property {string} apiUrl - The API URL for fetching email content.
      * @property {string} statusToCheck - The status to check against. Used for show/hide on textAreaFormGroup/dropdownFormGroup.
+     * @property {string} readonlyStatusToCheck - The status to check against when readonly. Used for show/hide on textAreaFormGroup/dropdownFormGroup.
      * @property {string} sessionVariableName - The session variable name. Used for show/hide on textAreaFormGroup/dropdownFormGroup.
      * @property {string} apiErrorMessage - The error message that the ajax call returns.
      */
@@ -338,6 +353,7 @@ class CustomizableEmailBase {
         this.textAreaFormGroup = config.textAreaFormGroup;
         this.dropdownFormGroup = config.dropdownFormGroup;
         this.statusToCheck = config.statusToCheck;
+        this.readonlyStatusToCheck = config.readonlyStatusToCheck;
         this.sessionVariableName = config.sessionVariableName;
 
         // Non-configurable variables
@@ -363,19 +379,31 @@ class CustomizableEmailBase {
 
     // Handle showing/hiding the related fields on page load.
     initializeFormGroups() {
-        let isStatus = this.statusSelect.value == this.statusToCheck;
+        let isStatus = false;
+        if (this.statusSelect) {
+            this.statusSelect.value == this.statusToCheck;
+        } else {
+            // statusSelect does not exist, indicating readonly
+            if (this.dropdownFormGroup) {
+                let readonlyDiv = this.dropdownFormGroup.querySelector("div.readonly");
+                let readonlyStatusText = readonlyDiv.textContent.trim();
+                isStatus = readonlyStatusText == this.readonlyStatusToCheck;
+            }
+        }
 
         // Initial handling of these groups.
         this.updateFormGroupVisibility(isStatus);
 
-        // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage
-        this.statusSelect.addEventListener('change', () => {
-            // Show the action needed field if the status is what we expect.
-            // Then track if its shown or hidden in our session cache.
-            isStatus = this.statusSelect.value == this.statusToCheck;
-            this.updateFormGroupVisibility(isStatus);
-            addOrRemoveSessionBoolean(this.sessionVariableName, isStatus);
-        });
+        if (this.statusSelect) {
+            // Listen to change events and handle rejectionReasonFormGroup display, then save status to session storage
+            this.statusSelect.addEventListener('change', () => {
+                // Show the action needed field if the status is what we expect.
+                // Then track if its shown or hidden in our session cache.
+                isStatus = this.statusSelect.value == this.statusToCheck;
+                this.updateFormGroupVisibility(isStatus);
+                addOrRemoveSessionBoolean(this.sessionVariableName, isStatus);
+            });
+        }
         
         // Listen to Back/Forward button navigation and handle rejectionReasonFormGroup display based on session storage
         // When you navigate using forward/back after changing status but not saving, when you land back on the DA page the
@@ -403,58 +431,64 @@ class CustomizableEmailBase {
     }
 
     initializeDropdown() {
-        this.dropdown.addEventListener("change", () => {
-            let reason = this.dropdown.value;
-            if (this.initialDropdownValue !== this.dropdown.value || this.initialEmailValue !== this.textarea.value) {
-                let searchParams = new URLSearchParams(
-                    {
-                        "reason": reason,
-                        "domain_request_id": this.domainRequestId,
-                    }
-                );
-                // Replace the email content
-                fetch(`${this.apiUrl}?${searchParams.toString()}`)
-                .then(response => {
-                    return response.json().then(data => data);
-                })
-                .then(data => {
-                    if (data.error) {
-                        console.error("Error in AJAX call: " + data.error);
-                    }else {
-                        this.textarea.value = data.email;
-                    }
-                    this.updateUserInterface(reason);
-                })
-                .catch(error => {
-                    console.error(this.apiErrorMessage, error)
-                });
-            }
-        });
+        if (this.dropdown) {
+            this.dropdown.addEventListener("change", () => {
+                let reason = this.dropdown.value;
+                if (this.initialDropdownValue !== this.dropdown.value || this.initialEmailValue !== this.textarea.value) {
+                    let searchParams = new URLSearchParams(
+                        {
+                            "reason": reason,
+                            "domain_request_id": this.domainRequestId,
+                        }
+                    );
+                    // Replace the email content
+                    fetch(`${this.apiUrl}?${searchParams.toString()}`)
+                    .then(response => {
+                        return response.json().then(data => data);
+                    })
+                    .then(data => {
+                        if (data.error) {
+                            console.error("Error in AJAX call: " + data.error);
+                        }else {
+                            this.textarea.value = data.email;
+                        }
+                        this.updateUserInterface(reason);
+                    })
+                    .catch(error => {
+                        console.error(this.apiErrorMessage, error)
+                    });
+                }
+            });
+        }
     }
 
     initializeModalConfirm() {
-        this.modalConfirm.addEventListener("click", () => {
-            this.textarea.removeAttribute('readonly');
-            this.textarea.focus();
-                hideElement(this.directEditButton);
-                hideElement(this.modalTrigger);  
-        });
+        if (this.modalConfirm) {
+            this.modalConfirm.addEventListener("click", () => {
+                this.textarea.removeAttribute('readonly');
+                this.textarea.focus();
+                    hideElement(this.directEditButton);
+                    hideElement(this.modalTrigger);  
+            });
+        }
     }
 
     initializeDirectEditButton() {
-        this.directEditButton.addEventListener("click", () => {
-            this.textarea.removeAttribute('readonly');
-            this.textarea.focus();
-                hideElement(this.directEditButton);
-                hideElement(this.modalTrigger);  
-        });
+        if (this.directEditButton) {
+            this.directEditButton.addEventListener("click", () => {
+                this.textarea.removeAttribute('readonly');
+                this.textarea.focus();
+                    hideElement(this.directEditButton);
+                    hideElement(this.modalTrigger);  
+            });
+        }
     }
 
     isEmailAlreadySent() {
         return this.lastSentEmailContent.value.replace(/\s+/g, '') === this.textarea.value.replace(/\s+/g, '');
     }
 
-    updateUserInterface(reason=this.dropdown.value, excluded_reasons=["other"]) {
+    updateUserInterface(reason, excluded_reasons=["other"]) {
         if (!reason) {
             // No reason selected, we will set the label to "Email", show the "Make a selection" placeholder, hide the trigger, textarea, hide the help text
             this.showPlaceholderNoReason();
@@ -468,23 +502,25 @@ class CustomizableEmailBase {
 
     // Helper function that makes overriding the readonly textarea easy
     showReadonlyTextarea() {
-        // A triggering selection is selected, all hands on board:
-        this.textarea.setAttribute('readonly', true);
-        showElement(this.textarea);
-        hideElement(this.textareaPlaceholder);
+        if (this.textarea && this.textareaPlaceholder) {
+            // A triggering selection is selected, all hands on board:
+            this.textarea.setAttribute('readonly', true);
+            showElement(this.textarea);
+            hideElement(this.textareaPlaceholder);
 
-            if (this.isEmailAlreadySentConst) {
-                hideElement(this.directEditButton);
-                showElement(this.modalTrigger);
+                if (this.isEmailAlreadySentConst) {
+                    hideElement(this.directEditButton);
+                    showElement(this.modalTrigger);
+                } else {
+                    showElement(this.directEditButton);
+                    hideElement(this.modalTrigger);
+            }
+
+            if (this.isEmailAlreadySent()) {
+                this.formLabel.innerHTML = "Email sent to creator:";
             } else {
-                showElement(this.directEditButton);
-                hideElement(this.modalTrigger);
-        }
-
-        if (this.isEmailAlreadySent()) {
-            this.formLabel.innerHTML = "Email sent to creator:";
-        } else {
-            this.formLabel.innerHTML = "Email:";
+                this.formLabel.innerHTML = "Email:";
+            }
         }
     }
 
@@ -516,9 +552,10 @@ class customActionNeededEmail extends CustomizableEmailBase {
             lastSentEmailContent: document.getElementById("last-sent-action-needed-email-content"),
             modalConfirm: document.getElementById("action-needed-reason__confirm-edit-email"),
             apiUrl: document.getElementById("get-action-needed-email-for-user-json")?.value || null,
-            textAreaFormGroup: document.querySelector('.field-action_needed_reason'),
-            dropdownFormGroup: document.querySelector('.field-action_needed_reason_email'),
+            textAreaFormGroup: document.querySelector('.field-action_needed_reason_email'),
+            dropdownFormGroup: document.querySelector('.field-action_needed_reason'),
             statusToCheck: "action needed",
+            readonlyStatusToCheck: "Action needed",
             sessionVariableName: "showActionNeededReason",
             apiErrorMessage: "Error when attempting to grab action needed email: "
         }
@@ -529,7 +566,15 @@ class customActionNeededEmail extends CustomizableEmailBase {
         // Hide/show the email fields depending on the current status
         this.initializeFormGroups();
         // Setup the textarea, edit button, helper text
-        this.updateUserInterface();
+        let reason = null;
+        if (this.dropdown) {
+            reason = this.dropdown.value;
+        } else if (this.dropdownFormGroup && this.dropdownFormGroup.querySelector("div.readonly")) {
+            if (this.dropdownFormGroup.querySelector("div.readonly").textContent) {
+                reason = this.dropdownFormGroup.querySelector("div.readonly").textContent.trim()
+            }
+        }
+        this.updateUserInterface(reason);
         this.initializeDropdown();
         this.initializeModalConfirm();
         this.initializeDirectEditButton();
@@ -560,12 +605,12 @@ export function initActionNeededEmail() {
         // Initialize UI
         const customEmail = new customActionNeededEmail();
 
-        // Check that every variable was setup correctly
-        const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
-        if (nullItems.length > 0) {
-            console.error(`Failed to load customActionNeededEmail(). Some variables were null: ${nullItems.join(", ")}`)
-            return;
-        }
+        // // Check that every variable was setup correctly
+        // const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
+        // if (nullItems.length > 0) {
+        //     console.error(`Failed to load customActionNeededEmail(). Some variables were null: ${nullItems.join(", ")}`)
+        //     return;
+        // }
         customEmail.loadActionNeededEmail()
     });
 }
@@ -581,6 +626,7 @@ class customRejectedEmail extends CustomizableEmailBase {
             textAreaFormGroup: document.querySelector('.field-rejection_reason'),
             dropdownFormGroup: document.querySelector('.field-rejection_reason_email'),
             statusToCheck: "rejected",
+            readonlyStatusToCheck: "Rejected",
             sessionVariableName: "showRejectionReason",
             errorMessage: "Error when attempting to grab rejected email: "
         };
@@ -589,7 +635,15 @@ class customRejectedEmail extends CustomizableEmailBase {
 
     loadRejectedEmail() {
         this.initializeFormGroups();
-        this.updateUserInterface();
+        let reason = null;
+        if (this.dropdown) {
+            reason = this.dropdown.value;
+        } else if (this.dropdownFormGroup && this.dropdownFormGroup.querySelector("div.readonly")) {
+            if (this.dropdownFormGroup.querySelector("div.readonly").textContent) {
+                reason = this.dropdownFormGroup.querySelector("div.readonly").textContent.trim()
+            }
+        }
+        this.updateUserInterface(reason);
         this.initializeDropdown();
         this.initializeModalConfirm();
         this.initializeDirectEditButton();
@@ -600,7 +654,7 @@ class customRejectedEmail extends CustomizableEmailBase {
         this.showPlaceholder("Email:", "Select a rejection reason to see email");
     }
 
-    updateUserInterface(reason=this.dropdown.value, excluded_reasons=[]) {
+    updateUserInterface(reason, excluded_reasons=[]) {
         super.updateUserInterface(reason, excluded_reasons);
     }
 }
@@ -619,12 +673,12 @@ export function initRejectedEmail() {
 
         // Initialize UI
         const customEmail = new customRejectedEmail();
-        // Check that every variable was setup correctly
-        const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
-        if (nullItems.length > 0) {
-            console.error(`Failed to load customRejectedEmail(). Some variables were null: ${nullItems.join(", ")}`)
-            return;
-        }
+        // // Check that every variable was setup correctly
+        // const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
+        // if (nullItems.length > 0) {
+        //     console.error(`Failed to load customRejectedEmail(). Some variables were null: ${nullItems.join(", ")}`)
+        //     return;
+        // }
         customEmail.loadRejectedEmail()
     });
 }
@@ -648,7 +702,6 @@ function handleSuborgFieldsAndButtons() {
 
     // Ensure that every variable is present before proceeding
     if (!requestedSuborganizationField || !suborganizationCity || !suborganizationStateTerritory || !rejectButton) {
-        console.warn("handleSuborganizationSelection() => Could not find required fields.")
         return;
     }
 
diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
index 9a60e1684..54d0e073b 100644
--- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
+++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
@@ -404,7 +404,7 @@ export function handlePortfolioSelection(
             updateSubOrganizationDropdown(portfolio_id);
 
             // Show fields relevant to a selected portfolio
-            showElement(suborganizationField);
+            if (suborganizationField) showElement(suborganizationField);
             hideElement(seniorOfficialField);
             showElement(portfolioSeniorOfficialField);
 
@@ -427,7 +427,7 @@ export function handlePortfolioSelection(
             // No portfolio is selected - reverse visibility of fields
 
             // Hide suborganization field as no portfolio is selected
-            hideElement(suborganizationField);
+            if (suborganizationField) hideElement(suborganizationField);
 
             // Show fields that are relevant when no portfolio is selected
             showElement(seniorOfficialField);

From 6ef3f50a7bcccd0157d81fa2566c517bf6467050 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 6 Mar 2025 22:46:12 -0500
Subject: [PATCH 13/47] fix portfolio javascript

---
 .../assets/src/js/getgov-admin/portfolio-form.js          | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
index 74729c2b2..2e437c520 100644
--- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
@@ -285,9 +285,11 @@ function handlePortfolioFields(){
                 handleStateTerritoryChange();
             });
         }
-        organizationTypeDropdown.addEventListener("change", function() {
-            handleOrganizationTypeChange();
-        });
+        if (organizationTypeDropdown) {
+            organizationTypeDropdown.addEventListener("change", function() {
+                handleOrganizationTypeChange();
+            });
+        }
     }
 
     // Run initial setup functions

From 952f1c35cc052dda6ccf21a15dcc2635efc44bed Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 7 Mar 2025 07:04:39 -0500
Subject: [PATCH 14/47] fixed place and remove hold

---
 src/registrar/admin.py                                    | 8 +++++---
 .../templates/django/admin/domain_change_form.html        | 2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index d9b9509e1..020dc2540 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -4329,9 +4329,11 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         # Fixes a bug wherein users which are only is_staff
         # can access 'change' when GET,
         # but cannot access this page when it is a request of type POST.
-        if request.user.has_perm("registrar.full_access_permission") or request.user.has_perm(
-            "registrar.analyst_access_permission"
-        ) or request.user.has_perm("registrar.omb_analyst_access_permission"):
+        if (
+            request.user.has_perm("registrar.full_access_permission")
+            or request.user.has_perm("registrar.analyst_access_permission")
+            or request.user.has_perm("registrar.omb_analyst_access_permission")
+        ):
             return True
         return super().has_change_permission(request, obj)
 
diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html
index 9f34feae6..2e6f57237 100644
--- a/src/registrar/templates/django/admin/domain_change_form.html
+++ b/src/registrar/templates/django/admin/domain_change_form.html
@@ -33,8 +33,10 @@
             <input type="submit" value="Remove hold" name="_remove_client_hold" class="custom-link-button">
             {% endif %}
             {% if original.state == original.State.READY or original.state == original.State.ON_HOLD %}
+            {% if not adminform.form.is_omb_analyst %}
             <span class="margin-left-05 margin-right-05 text-middle"> | </span>
             {% endif %}
+            {% endif %}
             {% if original.state != original.State.DELETED and not adminform.form.is_omb_analyst %}
             <a class="text-middle" href="#toggle-remove-from-registry" aria-controls="toggle-remove-from-registry" data-open-modal>
                 Remove from registry

From 7994484fed1d15e308a5ce16b0bcd6fe0b1be43c Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 7 Mar 2025 19:36:24 -0500
Subject: [PATCH 15/47] DomainInvitationAdmin tests

---
 src/registrar/admin.py            |   2 +-
 src/registrar/tests/common.py     |  19 +++++
 src/registrar/tests/test_admin.py | 125 ++++++++++++++++++++++++++++--
 3 files changed, 140 insertions(+), 6 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 020dc2540..c72c5271a 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1820,7 +1820,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
             if request.user.groups.filter(name="omb_analysts_group").exists():
                 return (
                     obj.domain.domain_info.converted_generic_org_type == DomainRequest.OrganizationChoices.FEDERAL
-                    and obj.domain.domain_info.federal_type == BranchChoices.EXECUTIVE
+                    and obj.domain.domain_info.converted_federal_type == BranchChoices.EXECUTIVE
                 )
         return super().has_view_permission(request, obj)
 
diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index bb65ef6b1..7b6068c6a 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -1009,6 +1009,25 @@ def create_user(**kwargs):
     user.groups.set([group])
     return user
 
+def create_omb_analyst_user(**kwargs):
+    """Creates a analyst user with is_staff=True and the group cisa_analysts_group"""
+    User = get_user_model()
+    p = "userpass"
+    user = User.objects.create_user(
+        username=kwargs.get("username", "ombanalystuser"),
+        email=kwargs.get("email", "ombanalyst@example.com"),
+        first_name=kwargs.get("first_name", "first"),
+        last_name=kwargs.get("last_name", "last"),
+        is_staff=kwargs.get("is_staff", True),
+        title=kwargs.get("title", "title"),
+        password=kwargs.get("password", p),
+        phone=kwargs.get("phone", "8003111234"),
+    )
+    # Retrieve the group or create it if it doesn't exist
+    group, _ = UserGroup.objects.get_or_create(name="omb_analysts_group")
+    # Add the user to the group
+    user.groups.set([group])
+    return user
 
 def create_test_user():
     username = "test_user"
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 1de6b1be3..3f2edbafb 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3,6 +3,7 @@ from django.utils import timezone
 from django.test import TestCase, RequestFactory, Client
 from django.contrib.admin.sites import AdminSite
 from registrar import models
+from registrar.utility.constants import BranchChoices
 from registrar.utility.email import EmailSendingError
 from registrar.utility.errors import MissingEmailError
 from waffle.testutils import override_flag
@@ -57,6 +58,7 @@ from .common import (
     MockDbForSharedTests,
     AuditedAdminMockData,
     completed_domain_request,
+    create_omb_analyst_user,
     create_test_user,
     generic_domain_object,
     less_console_noise,
@@ -136,18 +138,21 @@ class TestDomainInvitationAdmin(WebTest):
     csrf_checks = False
 
     @classmethod
-    def setUpClass(self):
+    def setUpClass(cls):
         super().setUpClass()
-        self.site = AdminSite()
-        self.factory = RequestFactory()
-        self.superuser = create_superuser()
+        cls.site = AdminSite()
+        cls.factory = RequestFactory()
 
     def setUp(self):
         super().setUp()
+        self.superuser = create_superuser()
+        self.cisa_analyst = create_user()
+        self.omb_analyst = create_omb_analyst_user()
         self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite())
         self.domain = Domain.objects.create(name="example.com")
+        self.fed_agency = FederalAgency.objects.create(agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE)
         self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser)
-        DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser)
+        self.domain_info = DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser)
         """Create a client object"""
         self.client = Client(HTTP_HOST="localhost:8080")
         self.client.force_login(self.superuser)
@@ -159,10 +164,120 @@ class TestDomainInvitationAdmin(WebTest):
         DomainInvitation.objects.all().delete()
         DomainInformation.objects.all().delete()
         Portfolio.objects.all().delete()
+        self.fed_agency.delete()
         Domain.objects.all().delete()
         Contact.objects.all().delete()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure regular analysts can view domain invitations."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.client.force_login(self.cisa_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, invitation.email)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view_non_feb_domain(self):
+        """Ensure OMB analysts cannot view non-federal domains."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"))
+        self.assertNotContains(response, invitation.email)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view_feb_domain(self):
+        """Ensure OMB analysts can view federal executive branch domains."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.portfolio.organization_type = DomainRequest.OrganizationChoices.FEDERAL
+        self.portfolio.federal_agency = self.fed_agency
+        self.portfolio.save()
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"))
+        self.assertContains(response, invitation.email)
+
+    @less_console_noise_decorator      
+    def test_superuser_view(self):
+        """Ensure superusers can view domain invitations."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, invitation.email)
+
+    @less_console_noise_decorator
+    def test_analyst_change(self):
+        """Ensure regular analysts can view domain invitations but not update."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.client.force_login(self.cisa_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_change", args=[invitation.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, invitation.email)
+        # test whether fields are readonly or editable
+        self.assertNotContains(response, "id_domain")
+        self.assertNotContains(response, "id_email")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change_non_feb_domain(self):
+        """Ensure OMB analysts cannot change non-federal domains."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_change", args=[invitation.id]))
+        self.assertEqual(response.status_code, 302)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change_feb_domain(self):
+        """Ensure OMB analysts can view federal executive branch domains."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        # update domain 
+        self.portfolio.organization_type = DomainRequest.OrganizationChoices.FEDERAL
+        self.portfolio.federal_agency = self.fed_agency
+        self.portfolio.save()
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_change", args=[invitation.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, invitation.email)
+        # test whether fields are readonly or editable
+        self.assertNotContains(response, "id_domain")
+        self.assertNotContains(response, "id_email")
+
+    @less_console_noise_decorator
+    def test_superuser_change(self):
+        """Ensure superusers can change domain invitations."""
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_change", args=[invitation.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, invitation.email)
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_domain")
+        self.assertContains(response, "id_email")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_filter_feb_domain(self):
+        """Ensure OMB analysts can apply filters and only federal executive branch domains show."""
+        # create invitation on domain that is not FEB
+        invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"), {"status": DomainInvitation.DomainInvitationStatus.INVITED})
+        self.assertNotContains(response, invitation.email)
+        # update domain 
+        self.portfolio.organization_type = DomainRequest.OrganizationChoices.FEDERAL
+        self.portfolio.federal_agency = self.fed_agency
+        self.portfolio.save()
+        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"), {"status": DomainInvitation.DomainInvitationStatus.INVITED})
+        self.assertContains(response, invitation.email)
+
+    # test_analyst_view
+    # test_omb_analyst_view_non_feb_domain
+    # test_omb_analyst_view_feb_domain
+    # test_superuser_view
+    # test_analyst_change
+    # test_omb_analyst_change_non_feb_domain
+    # test_omb_analyst_change_feb_domain
+    # test_superuser
+    # test_filter_feb
+
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From 394147d657bca5371a9be53d04791ab1a2e8d638 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 7 Mar 2025 19:51:40 -0500
Subject: [PATCH 16/47] DomainInvitationAdmin tests

---
 src/registrar/tests/common.py     |  2 ++
 src/registrar/tests/test_admin.py | 34 +++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index 7b6068c6a..622bd5b22 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -1009,6 +1009,7 @@ def create_user(**kwargs):
     user.groups.set([group])
     return user
 
+
 def create_omb_analyst_user(**kwargs):
     """Creates a analyst user with is_staff=True and the group cisa_analysts_group"""
     User = get_user_model()
@@ -1029,6 +1030,7 @@ def create_omb_analyst_user(**kwargs):
     user.groups.set([group])
     return user
 
+
 def create_test_user():
     username = "test_user"
     first_name = "First"
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 3f2edbafb..a21bfd4f2 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -150,9 +150,13 @@ class TestDomainInvitationAdmin(WebTest):
         self.omb_analyst = create_omb_analyst_user()
         self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite())
         self.domain = Domain.objects.create(name="example.com")
-        self.fed_agency = FederalAgency.objects.create(agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE)
+        self.fed_agency = FederalAgency.objects.create(
+            agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE
+        )
         self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser)
-        self.domain_info = DomainInformation.objects.create(domain=self.domain, portfolio=self.portfolio, creator=self.superuser)
+        self.domain_info = DomainInformation.objects.create(
+            domain=self.domain, portfolio=self.portfolio, creator=self.superuser
+        )
         """Create a client object"""
         self.client = Client(HTTP_HOST="localhost:8080")
         self.client.force_login(self.superuser)
@@ -197,7 +201,7 @@ class TestDomainInvitationAdmin(WebTest):
         response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"))
         self.assertContains(response, invitation.email)
 
-    @less_console_noise_decorator      
+    @less_console_noise_decorator
     def test_superuser_view(self):
         """Ensure superusers can view domain invitations."""
         invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
@@ -216,6 +220,9 @@ class TestDomainInvitationAdmin(WebTest):
         # test whether fields are readonly or editable
         self.assertNotContains(response, "id_domain")
         self.assertNotContains(response, "id_email")
+        self.assertContains(response, "closelink")
+        self.assertNotContains(response, "Save")
+        self.assertNotContains(response, "Delete")
 
     @less_console_noise_decorator
     def test_omb_analyst_change_non_feb_domain(self):
@@ -229,7 +236,7 @@ class TestDomainInvitationAdmin(WebTest):
     def test_omb_analyst_change_feb_domain(self):
         """Ensure OMB analysts can view federal executive branch domains."""
         invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
-        # update domain 
+        # update domain
         self.portfolio.organization_type = DomainRequest.OrganizationChoices.FEDERAL
         self.portfolio.federal_agency = self.fed_agency
         self.portfolio.save()
@@ -240,6 +247,9 @@ class TestDomainInvitationAdmin(WebTest):
         # test whether fields are readonly or editable
         self.assertNotContains(response, "id_domain")
         self.assertNotContains(response, "id_email")
+        self.assertContains(response, "closelink")
+        self.assertNotContains(response, "Save")
+        self.assertNotContains(response, "Delete")
 
     @less_console_noise_decorator
     def test_superuser_change(self):
@@ -251,6 +261,9 @@ class TestDomainInvitationAdmin(WebTest):
         # test whether fields are readonly or editable
         self.assertContains(response, "id_domain")
         self.assertContains(response, "id_email")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
 
     @less_console_noise_decorator
     def test_omb_analyst_filter_feb_domain(self):
@@ -258,13 +271,19 @@ class TestDomainInvitationAdmin(WebTest):
         # create invitation on domain that is not FEB
         invitation = DomainInvitation.objects.create(email="test@example.com", domain=self.domain)
         self.client.force_login(self.omb_analyst)
-        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"), {"status": DomainInvitation.DomainInvitationStatus.INVITED})
+        response = self.client.get(
+            reverse("admin:registrar_domaininvitation_changelist"),
+            {"status": DomainInvitation.DomainInvitationStatus.INVITED},
+        )
         self.assertNotContains(response, invitation.email)
-        # update domain 
+        # update domain
         self.portfolio.organization_type = DomainRequest.OrganizationChoices.FEDERAL
         self.portfolio.federal_agency = self.fed_agency
         self.portfolio.save()
-        response = self.client.get(reverse("admin:registrar_domaininvitation_changelist"), {"status": DomainInvitation.DomainInvitationStatus.INVITED})
+        response = self.client.get(
+            reverse("admin:registrar_domaininvitation_changelist"),
+            {"status": DomainInvitation.DomainInvitationStatus.INVITED},
+        )
         self.assertContains(response, invitation.email)
 
     # test_analyst_view
@@ -277,7 +296,6 @@ class TestDomainInvitationAdmin(WebTest):
     # test_superuser
     # test_filter_feb
 
-
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From fb4e278ba5e3cbaafc5bc5f348f5c7ececc59f19 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 7 Mar 2025 20:14:44 -0500
Subject: [PATCH 17/47] UserPortfolioPermissionAdmin tests

---
 src/registrar/tests/test_admin.py | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index a21bfd4f2..e17c311ec 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -286,16 +286,6 @@ class TestDomainInvitationAdmin(WebTest):
         )
         self.assertContains(response, invitation.email)
 
-    # test_analyst_view
-    # test_omb_analyst_view_non_feb_domain
-    # test_omb_analyst_view_feb_domain
-    # test_superuser_view
-    # test_analyst_change
-    # test_omb_analyst_change_non_feb_domain
-    # test_omb_analyst_change_feb_domain
-    # test_superuser
-    # test_filter_feb
-
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
@@ -1272,6 +1262,7 @@ class TestUserPortfolioPermissionAdmin(TestCase):
         self.client = Client(HTTP_HOST="localhost:8080")
         self.superuser = create_superuser()
         self.testuser = create_test_user()
+        self.omb_analyst = create_omb_analyst_user()
         self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser)
 
     def tearDown(self):
@@ -1281,6 +1272,26 @@ class TestUserPortfolioPermissionAdmin(TestCase):
         User.objects.all().delete()
         UserPortfolioPermission.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view user portfolio permissions list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_userportfoliopermission_changelist"))
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts cannot change user portfolio permission."""
+        self.client.force_login(self.omb_analyst)
+        user_portfolio_permission, _ = UserPortfolioPermission.objects.get_or_create(
+            user=self.superuser, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+        response = self.client.get(
+            "/admin/registrar/userportfoliopermission/{}/change/".format(user_portfolio_permission.pk),
+            follow=True,
+        )
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_change_form_description(self):
         """Tests if this model has a model description on the change form view"""

From a2091c54958486d0fa56b3f3b2c412a7b12e1c25 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Fri, 7 Mar 2025 20:21:12 -0500
Subject: [PATCH 18/47] PortfolioInvitationAdmin tests

---
 src/registrar/tests/test_admin.py | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index e17c311ec..156e8566a 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1348,6 +1348,7 @@ class TestPortfolioInvitationAdmin(TestCase):
     def setUp(self):
         """Create a client object"""
         self.client = Client(HTTP_HOST="localhost:8080")
+        self.omb_analyst = create_omb_analyst_user()
         self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser)
 
     def tearDown(self):
@@ -1361,6 +1362,26 @@ class TestPortfolioInvitationAdmin(TestCase):
     def tearDownClass(self):
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view portfolio invitations list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_portfolioinvitation_changelist"))
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts cannot change portfolio invitation."""
+        self.client.force_login(self.omb_analyst)
+        invitation, _ = PortfolioInvitation.objects.get_or_create(
+            email=self.superuser.email, portfolio=self.portfolio, roles=[UserPortfolioRoleChoices.ORGANIZATION_ADMIN]
+        )
+        response = self.client.get(
+            "/admin/registrar/portfolioinvitation/{}/change/".format(invitation.pk),
+            follow=True,
+        )
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From 3bf8333f3dd87546644b9a844ab976771257e01a Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 07:20:49 -0500
Subject: [PATCH 19/47] TestDomainInformationAdmin tests

---
 src/registrar/tests/test_admin.py | 71 +++++++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 156e8566a..f9b9b1980 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1956,6 +1956,7 @@ class TestHostAdmin(TestCase):
         cls.factory = RequestFactory()
         cls.admin = MyHostAdmin(model=Host, admin_site=cls.site)
         cls.superuser = create_superuser()
+        cls.omb_analyst = create_omb_analyst_user()
 
     def setUp(self):
         """Setup environment for a mock admin user"""
@@ -1971,6 +1972,13 @@ class TestHostAdmin(TestCase):
     def tearDownClass(cls):
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view hosts list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_host_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
@@ -2035,6 +2043,7 @@ class TestDomainInformationAdmin(TestCase):
         cls.admin = DomainInformationAdmin(model=DomainInformation, admin_site=cls.site)
         cls.superuser = create_superuser()
         cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.mock_data_generator = AuditedAdminMockData()
         cls.test_helper = GenericTestHelper(
             factory=cls.factory,
@@ -2046,12 +2055,24 @@ class TestDomainInformationAdmin(TestCase):
 
     def setUp(self):
         self.client = Client(HTTP_HOST="localhost:8080")
+        self.nonfeddomain = Domain.objects.create(name="nonfeddomain.com")
+        self.feddomain = Domain.objects.create(name="feddomain.com")
+        self.fed_agency = FederalAgency.objects.create(
+            agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.portfolio = Portfolio.objects.create(organization_name="new portfolio", creator=self.superuser)
+        self.domain_info = DomainInformation.objects.create(
+            domain=self.feddomain, portfolio=self.portfolio, creator=self.superuser
+        )
 
     def tearDown(self):
         """Delete all Users, Domains, and UserDomainRoles"""
         DomainInformation.objects.all().delete()
         DomainRequest.objects.all().delete()
         Domain.objects.all().delete()
+        DomainInformation.objects.all().delete()
+        Portfolio.objects.all().delete()
+        self.fed_agency.delete()
         Contact.objects.all().delete()
 
     @classmethod
@@ -2059,6 +2080,56 @@ class TestDomainInformationAdmin(TestCase):
         User.objects.all().delete()
         SeniorOfficial.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure regular analysts cannot view domain information list."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_domaininformation_changelist"))
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view domain information list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domaininformation_changelist"))
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_superuser_view(self):
+        """Ensure superusers can view domain information list."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_domaininformation_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feddomain.name)
+
+    @less_console_noise_decorator
+    def test_analyst_change(self):
+        """Ensure regular analysts cannot view/edit domain information directly."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(
+            reverse("admin:registrar_domaininformation_change", args=[self.feddomain.domain_info.id])
+        )
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts cannot view/edit domain information directly."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(
+            reverse("admin:registrar_domaininformation_change", args=[self.feddomain.domain_info.id])
+        )
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_superuser_change(self):
+        """Ensure superusers can view/change domain information directly."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(
+            reverse("admin:registrar_domaininformation_change", args=[self.feddomain.domain_info.id])
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feddomain.name)
+
     @less_console_noise_decorator
     def test_domain_information_senior_official_is_alphabetically_sorted(self):
         """Tests if the senior offical dropdown is alphanetically sorted in the django admin display"""

From b8636b3473ebbdf5d842a0589e1ec05a60bfac2e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 07:32:49 -0500
Subject: [PATCH 20/47] TestUserDomainRoleAdmin tests

---
 src/registrar/tests/test_admin.py | 35 +++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index f9b9b1980..ccb54747a 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -1956,6 +1956,7 @@ class TestHostAdmin(TestCase):
         cls.factory = RequestFactory()
         cls.admin = MyHostAdmin(model=Host, admin_site=cls.site)
         cls.superuser = create_superuser()
+        cls.staffuser = create_user()
         cls.omb_analyst = create_omb_analyst_user()
 
     def setUp(self):
@@ -1972,6 +1973,13 @@ class TestHostAdmin(TestCase):
     def tearDownClass(cls):
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure analysts cannot view hosts list."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_host_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_omb_analyst_view(self):
         """Ensure OMB analysts cannot view hosts list."""
@@ -2494,6 +2502,8 @@ class TestUserDomainRoleAdmin(WebTest):
         cls.factory = RequestFactory()
         cls.admin = UserDomainRoleAdmin(model=UserDomainRole, admin_site=cls.site)
         cls.superuser = create_superuser()
+        cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.test_helper = GenericTestHelper(
             factory=cls.factory,
             user=cls.superuser,
@@ -2521,6 +2531,31 @@ class TestUserDomainRoleAdmin(WebTest):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure analysts cannot view user domain roles list."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_userdomainrole_changelist"))
+        self.assertEqual(response.status_code, 200)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view user domain roles list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_userdomainrole_changelist"))
+        self.assertEqual(response.status_code, 403)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts cannot view/edit user domain roles list."""
+        domain, _ = Domain.objects.get_or_create(name="anyrandomdomain.com")
+        user_domain_role, _ = UserDomainRole.objects.get_or_create(
+            user=self.superuser, domain=domain, role=[UserDomainRole.Roles.MANAGER]
+        )
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_userdomainrole_change", args=[user_domain_role.id]))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From 8ef58f6053e38f7facdf0cc94e817c793fc23a69 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 07:41:38 -0500
Subject: [PATCH 21/47] TestNyUserAdmin and ContactAdmin tests

---
 src/registrar/tests/test_admin.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index ccb54747a..214160c14 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2851,6 +2851,7 @@ class TestMyUserAdmin(MockDbForSharedTests, WebTest):
         cls.admin = MyUserAdmin(model=get_user_model(), admin_site=admin_site)
         cls.superuser = create_superuser()
         cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.test_helper = GenericTestHelper(admin=cls.admin)
 
     def setUp(self):
@@ -2867,6 +2868,13 @@ class TestMyUserAdmin(MockDbForSharedTests, WebTest):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view users list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_user_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
@@ -3492,6 +3500,7 @@ class TestContactAdmin(TestCase):
         cls.admin = ContactAdmin(model=Contact, admin_site=None)
         cls.superuser = create_superuser()
         cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
 
     def setUp(self):
         super().setUp()
@@ -3507,6 +3516,13 @@ class TestContactAdmin(TestCase):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view contact list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_contact_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From 67ed777900446205e1e90ec2f7245f694c8148b2 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 07:53:51 -0500
Subject: [PATCH 22/47] DraftDomain Website and VerifiedByStaff Admin tests

---
 src/registrar/tests/test_admin.py | 45 +++++++++++++++++++------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 214160c14..f8358a966 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3569,6 +3569,7 @@ class TestVerifiedByStaffAdmin(TestCase):
         super().setUpClass()
         cls.site = AdminSite()
         cls.superuser = create_superuser()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.admin = VerifiedByStaffAdmin(model=VerifiedByStaff, admin_site=cls.site)
         cls.factory = RequestFactory()
         cls.test_helper = GenericTestHelper(admin=cls.admin)
@@ -3586,18 +3587,20 @@ class TestVerifiedByStaffAdmin(TestCase):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view verified by staff list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_verifiedbystaff_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         self.client.force_login(self.superuser)
-        response = self.client.get(
-            "/admin/registrar/verifiedbystaff/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_verifiedbystaff_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
-
         # Test for a description snippet
         self.assertContains(
             response, "This table contains users who have been allowed to bypass " "identity proofing through Login.gov"
@@ -3652,6 +3655,7 @@ class TestWebsiteAdmin(TestCase):
         super().setUp()
         self.site = AdminSite()
         self.superuser = create_superuser()
+        self.omb_analyst = create_omb_analyst_user()
         self.admin = WebsiteAdmin(model=Website, admin_site=self.site)
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
@@ -3662,15 +3666,18 @@ class TestWebsiteAdmin(TestCase):
         Website.objects.all().delete()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view website list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_website_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         self.client.force_login(self.superuser)
-        response = self.client.get(
-            "/admin/registrar/website/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_website_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
 
@@ -3679,13 +3686,14 @@ class TestWebsiteAdmin(TestCase):
         self.assertContains(response, "Show more")
 
 
-class TestDraftDomain(TestCase):
+class TestDraftDomainAdmin(TestCase):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
         cls.site = AdminSite()
         cls.superuser = create_superuser()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.admin = DraftDomainAdmin(model=DraftDomain, admin_site=cls.site)
         cls.factory = RequestFactory()
         cls.test_helper = GenericTestHelper(admin=cls.admin)
@@ -3703,15 +3711,18 @@ class TestDraftDomain(TestCase):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view draft domain list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_draftdomain_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         self.client.force_login(self.superuser)
-        response = self.client.get(
-            "/admin/registrar/draftdomain/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_draftdomain_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
 

From 17b7c83b4555481949849b889a78259aed9ab4a8 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 08:44:37 -0500
Subject: [PATCH 23/47] FederalAgency Admin tests

---
 src/registrar/tests/test_admin.py | 104 +++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 1 deletion(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index f8358a966..8a79ccba5 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3733,13 +3733,21 @@ class TestDraftDomainAdmin(TestCase):
         self.assertContains(response, "Show more")
 
 
-class TestFederalAgency(TestCase):
+class TestFederalAgencyAdmin(TestCase):
 
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
         cls.site = AdminSite()
         cls.superuser = create_superuser()
+        cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
+        cls.non_feb_agency = FederalAgency.objects.create(
+            agency="Fake judicial agency", federal_type=BranchChoices.JUDICIAL
+        )
+        cls.feb_agency = FederalAgency.objects.create(
+            agency="Fake executive agency", federal_type=BranchChoices.EXECUTIVE
+        )
         cls.admin = FederalAgencyAdmin(model=FederalAgency, admin_site=cls.site)
         cls.factory = RequestFactory()
         cls.test_helper = GenericTestHelper(admin=cls.admin)
@@ -3752,6 +3760,100 @@ class TestFederalAgency(TestCase):
         super().tearDownClass()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure regular analysts can view federal agencies."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_federalagency_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.non_feb_agency.agency)
+        self.assertContains(response, self.feb_agency.agency)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts can view FEB agencies but not other branches."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_federalagency_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertNotContains(response, self.non_feb_agency.agency)
+        self.assertContains(response, self.feb_agency.agency)
+
+    @less_console_noise_decorator
+    def test_superuser_view(self):
+        """Ensure superusers can view domain invitations."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_federalagency_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.non_feb_agency.agency)
+        self.assertContains(response, self.feb_agency.agency)
+
+    @less_console_noise_decorator
+    def test_analyst_change(self):
+        """Ensure regular analysts can view/edit federal agencies list."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.non_feb_agency.id]))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.feb_agency.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_agency.agency)
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_agency")
+        self.assertContains(response, "id_federal_type")
+        self.assertContains(response, "id_acronym")
+        self.assertContains(response, "id_is_fceb")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts can change FEB agencies but not others."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.non_feb_agency.id]))
+        self.assertEqual(response.status_code, 302)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.feb_agency.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_agency.agency)
+        # test whether fields are readonly or editable
+        self.assertNotContains(response, "id_agency")
+        self.assertNotContains(response, "id_federal_type")
+        self.assertNotContains(response, "id_acronym")
+        self.assertNotContains(response, "id_is_fceb")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_superuser_change(self):
+        """Ensure superusers can change all federal agencies."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.non_feb_agency.id]))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse("admin:registrar_federalagency_change", args=[self.feb_agency.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_agency.agency)
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_agency")
+        self.assertContains(response, "id_federal_type")
+        self.assertContains(response, "id_acronym")
+        self.assertContains(response, "id_is_fceb")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_filter_feb_agencies(self):
+        """Ensure OMB analysts can apply filters and only federal agencies show."""
+        self.client.force_login(self.omb_analyst)
+        # in setup, created two agencies: Fake judicial agency and Fake executive agency
+        # only executive agency should show up with the search for 'fake'
+        response = self.client.get(
+            reverse("admin:registrar_federalagency_changelist"),
+            data = {"q": "fake"},
+        )
+        self.assertNotContains(response, self.non_feb_agency.agency)
+        self.assertContains(response, self.feb_agency.agency)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""

From 8aff93c2f2009a9d342ed542eb63e7430db6d719 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 08:52:57 -0500
Subject: [PATCH 24/47] TransitionDomain UserGroup PublicContact Admin tests

---
 src/registrar/tests/test_admin.py | 50 +++++++++++++++++++------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 8a79ccba5..ced19b5b7 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3849,7 +3849,7 @@ class TestFederalAgencyAdmin(TestCase):
         # only executive agency should show up with the search for 'fake'
         response = self.client.get(
             reverse("admin:registrar_federalagency_changelist"),
-            data = {"q": "fake"},
+            data={"q": "fake"},
         )
         self.assertNotContains(response, self.non_feb_agency.agency)
         self.assertContains(response, self.feb_agency.agency)
@@ -3871,11 +3871,12 @@ class TestFederalAgencyAdmin(TestCase):
         self.assertContains(response, "Show more")
 
 
-class TestPublicContact(TestCase):
+class TestPublicContactAdmin(TestCase):
     def setUp(self):
         super().setUp()
         self.site = AdminSite()
         self.superuser = create_superuser()
+        self.omb_analyst = create_omb_analyst_user()
         self.admin = PublicContactAdmin(model=PublicContact, admin_site=self.site)
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
@@ -3886,16 +3887,19 @@ class TestPublicContact(TestCase):
         PublicContact.objects.all().delete()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view public contact list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_publiccontact_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         p = "adminpass"
         self.client.login(username="superuser", password=p)
-        response = self.client.get(
-            "/admin/registrar/publiccontact/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_publiccontact_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
 
@@ -3904,11 +3908,12 @@ class TestPublicContact(TestCase):
         self.assertContains(response, "Show more")
 
 
-class TestTransitionDomain(TestCase):
+class TestTransitionDomainAdmin(TestCase):
     def setUp(self):
         super().setUp()
         self.site = AdminSite()
         self.superuser = create_superuser()
+        self.omb_analyst = create_omb_analyst_user()
         self.admin = TransitionDomainAdmin(model=TransitionDomain, admin_site=self.site)
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
@@ -3919,15 +3924,18 @@ class TestTransitionDomain(TestCase):
         PublicContact.objects.all().delete()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view transition domain list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_transitiondomain_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         self.client.force_login(self.superuser)
-        response = self.client.get(
-            "/admin/registrar/transitiondomain/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_transitiondomain_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
 
@@ -3936,11 +3944,12 @@ class TestTransitionDomain(TestCase):
         self.assertContains(response, "Show more")
 
 
-class TestUserGroup(TestCase):
+class TestUserGroupAdmin(TestCase):
     def setUp(self):
         super().setUp()
         self.site = AdminSite()
         self.superuser = create_superuser()
+        self.omb_analyst = create_omb_analyst_user()
         self.admin = UserGroupAdmin(model=UserGroup, admin_site=self.site)
         self.factory = RequestFactory()
         self.client = Client(HTTP_HOST="localhost:8080")
@@ -3950,15 +3959,18 @@ class TestUserGroup(TestCase):
         super().tearDown()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts cannot view user group list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_usergroup_changelist"))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_has_model_description(self):
         """Tests if this model has a model description on the table view"""
         self.client.force_login(self.superuser)
-        response = self.client.get(
-            "/admin/registrar/usergroup/",
-            follow=True,
-        )
-
+        response = self.client.get(reverse("admin:registrar_usergroup_changelist"))
         # Make sure that the page is loaded correctly
         self.assertEqual(response.status_code, 200)
 

From 0708d54c48fa46f83ccfdf9acccdf99dc34203c3 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 09:52:36 -0500
Subject: [PATCH 25/47] Portfolio Admin tests

---
 src/registrar/tests/test_admin.py | 123 +++++++++++++++++++++++++++++-
 1 file changed, 122 insertions(+), 1 deletion(-)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index ced19b5b7..abf151184 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3987,12 +3987,23 @@ class TestPortfolioAdmin(TestCase):
         super().setUpClass()
         cls.site = AdminSite()
         cls.superuser = create_superuser()
+        cls.staffuser = create_user()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.admin = PortfolioAdmin(model=Portfolio, admin_site=cls.site)
         cls.factory = RequestFactory()
 
     def setUp(self):
         self.client = Client(HTTP_HOST="localhost:8080")
-        self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.superuser)
+        self.portfolio = Portfolio.objects.create(organization_name="Test portfolio", creator=self.superuser)
+        self.feb_agency = FederalAgency.objects.create(
+            agency="Test FedExec Agency", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.feb_portfolio = Portfolio.objects.create(
+            organization_name="Test FEB portfolio",
+            creator=self.superuser,
+            federal_agency=self.feb_agency,
+            organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+        )
 
     def tearDown(self):
         Suborganization.objects.all().delete()
@@ -4000,8 +4011,118 @@ class TestPortfolioAdmin(TestCase):
         DomainRequest.objects.all().delete()
         Domain.objects.all().delete()
         Portfolio.objects.all().delete()
+        self.feb_agency.delete()
         User.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_analyst_view(self):
+        """Ensure regular analysts can view portfolios."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_portfolio_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.portfolio.organization_name)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts can view FEB portfolios but not others."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_portfolio_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertNotContains(response, self.portfolio.organization_name)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+
+    @less_console_noise_decorator
+    def test_superuser_view(self):
+        """Ensure superusers can view portfolios."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_portfolio_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.portfolio.organization_name)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+
+    @less_console_noise_decorator
+    def test_analyst_change(self):
+        """Ensure regular analysts can view/edit portfolios."""
+        self.client.force_login(self.staffuser)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.portfolio.id]))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.feb_portfolio.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_organization_name")
+        self.assertContains(response, "id_notes")
+        self.assertContains(response, "id_organization_type")
+        self.assertContains(response, "id_state_territory")
+        self.assertContains(response, "id_address_line1")
+        self.assertContains(response, "id_address_line2")
+        self.assertContains(response, "id_city")
+        self.assertContains(response, "id_zipcode")
+        self.assertContains(response, "id_urbanization")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts can change FEB portfolios but not others."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.portfolio.id]))
+        self.assertEqual(response.status_code, 302)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.feb_portfolio.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+        # test whether fields are readonly or editable
+        self.assertNotContains(response, "id_organization_name")
+        self.assertNotContains(response, "id_notes")
+        self.assertNotContains(response, "id_organization_type")
+        self.assertNotContains(response, "id_state_territory")
+        self.assertNotContains(response, "id_address_line1")
+        self.assertNotContains(response, "id_address_line2")
+        self.assertNotContains(response, "id_city")
+        self.assertNotContains(response, "id_zipcode")
+        self.assertNotContains(response, "id_urbanization")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertNotContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_superuser_change(self):
+        """Ensure superusers can change all portfolios."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.portfolio.id]))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse("admin:registrar_portfolio_change", args=[self.feb_portfolio.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_organization_name")
+        self.assertContains(response, "id_notes")
+        self.assertContains(response, "id_organization_type")
+        self.assertContains(response, "id_state_territory")
+        self.assertContains(response, "id_address_line1")
+        self.assertContains(response, "id_address_line2")
+        self.assertContains(response, "id_city")
+        self.assertContains(response, "id_zipcode")
+        self.assertContains(response, "id_urbanization")
+        self.assertNotContains(response, "closelink")
+        self.assertContains(response, "Save")
+        self.assertContains(response, "Delete")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_filter_feb_portfolios(self):
+        """Ensure OMB analysts can apply filters and only feb portfolios show."""
+        self.client.force_login(self.omb_analyst)
+        # in setup, created two portfolios: Test portfolio and Test FEB portfolio
+        # only executive portfolio should show up with the search for 'portfolio'
+        response = self.client.get(
+            reverse("admin:registrar_portfolio_changelist"),
+            data={"q": "test"},
+        )
+        self.assertNotContains(response, self.portfolio.organization_name)
+        self.assertContains(response, self.feb_portfolio.organization_name)
+
     @less_console_noise_decorator
     def test_created_on_display(self):
         """Tests the custom created on which is a reskin of the created_at field"""

From b474e0eb6b6a2ca2fcb3abd99c17af2a5574504e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sat, 8 Mar 2025 09:57:53 -0500
Subject: [PATCH 26/47] Transfer User tests

---
 src/registrar/tests/test_admin.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index abf151184..e89a6352c 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -4310,6 +4310,7 @@ class TestTransferUser(WebTest):
         super().setUpClass()
         cls.site = AdminSite()
         cls.superuser = create_superuser()
+        cls.omb_analyst = create_omb_analyst_user()
         cls.admin = PortfolioAdmin(model=Portfolio, admin_site=cls.site)
         cls.factory = RequestFactory()
 
@@ -4330,6 +4331,13 @@ class TestTransferUser(WebTest):
         Portfolio.objects.all().delete()
         UserDomainRole.objects.all().delete()
 
+    @less_console_noise_decorator
+    def test_omb_analyst(self):
+        """Ensure OMB analysts cannot view transfer_user."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("transfer_user", args=[self.user1.pk]))
+        self.assertEqual(response.status_code, 403)
+
     @less_console_noise_decorator
     def test_transfer_user_shows_current_and_selected_user_information(self):
         """Assert we pull the current user info and display it on the transfer page"""

From 6702dca37dc7e8861ad5596f601738666a9ec4e5 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sun, 9 Mar 2025 10:39:49 -0400
Subject: [PATCH 27/47] wip

---
 src/registrar/admin.py                   | 18 +++++++
 src/registrar/tests/test_admin_domain.py | 64 ++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index c72c5271a..532b0b615 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -412,6 +412,17 @@ class DomainInformationAdminForm(forms.ModelForm):
 class DomainInformationInlineForm(forms.ModelForm):
     """This form utilizes the custom widget for its class's ManyToMany UIs."""
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+
+        # for OMB analysts, limit portfolio dropdown to FEB portfolios
+        user = self.request.user if hasattr(self, 'request') else None
+        if user and user.groups.filter(name="omb_analysts_group").exists():
+            self.fields["portfolio"].queryset = models.Portfolio.objects.filter(
+                Q(organization_type=DomainRequest.OrganizationChoices.FEDERAL) &
+                Q(federal_agency__federal_type=BranchChoices.EXECUTIVE)
+            )
+    
     class Meta:
         model = models.DomainInformation
         fields = "__all__"
@@ -2980,6 +2991,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         "rejection_reason_email",
         "action_needed_reason",
         "action_needed_reason_email",
+        "portfolio",
     ]
 
     autocomplete_fields = [
@@ -3753,6 +3765,12 @@ class DomainInformationInline(admin.StackedInline):
         form.is_omb_analyst = self.is_omb_analyst
 
         return form
+    
+    def get_formset(self, request, obj=None, **kwargs):
+        """Attach request to the formset so that it can be available in the form"""
+        formset = super().get_formset(request, obj, **kwargs)
+        formset.form.request = request  # Attach request to form
+        return formset
 
 
 class DomainResource(FsmModelResource):
diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py
index 969d043d7..2122c576f 100644
--- a/src/registrar/tests/test_admin_domain.py
+++ b/src/registrar/tests/test_admin_domain.py
@@ -17,14 +17,17 @@ from registrar.models import (
     Host,
     Portfolio,
 )
+from registrar.models.federal_agency import FederalAgency
 from registrar.models.public_contact import PublicContact
 from registrar.models.user_domain_role import UserDomainRole
+from registrar.utility.constants import BranchChoices
 from .common import (
     MockSESClient,
     completed_domain_request,
     less_console_noise,
     create_superuser,
     create_user,
+    create_omb_analyst_user,
     create_ready_domain,
     MockEppLib,
     GenericTestHelper,
@@ -49,6 +52,7 @@ class TestDomainAdminAsStaff(MockEppLib):
     def setUpClass(self):
         super().setUpClass()
         self.staffuser = create_user()
+        self.omb_analyst = create_omb_analyst_user()
         self.site = AdminSite()
         self.admin = DomainAdmin(model=Domain, admin_site=self.site)
         self.factory = RequestFactory()
@@ -56,6 +60,24 @@ class TestDomainAdminAsStaff(MockEppLib):
     def setUp(self):
         self.client = Client(HTTP_HOST="localhost:8080")
         self.client.force_login(self.staffuser)
+        self.nonfebdomain = Domain.objects.create(name="nonfebexample.com")
+        self.febdomain = Domain.objects.create(name="febexample.com", state=Domain.State.READY)
+        self.fed_agency = FederalAgency.objects.create(
+            agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.portfolio = Portfolio.objects.create(
+            organization_name="new portfolio",
+            organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+            federal_agency=self.fed_agency,
+            creator=self.staffuser,
+        )
+        self.domain_info = DomainInformation.objects.create(
+            domain=self.febdomain, portfolio=self.portfolio, creator=self.staffuser
+        )
+        self.nonfebportfolio = Portfolio.objects.create(
+            organization_name="non feb portfolio",
+            creator=self.staffuser,
+        )
         super().setUp()
 
     def tearDown(self):
@@ -65,12 +87,54 @@ class TestDomainAdminAsStaff(MockEppLib):
         Domain.objects.all().delete()
         DomainInformation.objects.all().delete()
         DomainRequest.objects.all().delete()
+        Portfolio.objects.all().delete()
+        self.fed_agency.delete()
 
     @classmethod
     def tearDownClass(self):
         User.objects.all().delete()
         super().tearDownClass()
 
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts can view domain list."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domain_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.febdomain.name)
+        self.assertNotContains(response, self.nonfebdomain.name)
+        self.assertNotContains(response, "Import")
+        self.assertNotContains(response, "Export")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts can view/edit federal executive branch domains."""
+        self.client.force_login(self.omb_analyst)
+        response = self.client.get(reverse("admin:registrar_domain_change", args=[self.nonfebdomain.id]))
+        self.assertEqual(response.status_code, 302)
+        response = self.client.get(reverse("admin:registrar_domain_change", args=[self.febdomain.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.febdomain.name)
+        # test portfolio dropdown
+        self.assertContains(response, self.portfolio.organization_name)
+        self.assertNotContains(response, self.nonfebportfolio.organization_name)
+        # test buttons
+        self.assertNotContains(response, "Manage domain")
+        self.assertNotContains(response, "Get registry status")
+        self.assertNotContains(response, "Extend expiration date")
+        self.assertNotContains(response, "Remove from registry")
+        self.assertContains(response, "Place hold")
+        self.assertContains(response, "Save")
+        self.assertNotContains(response, ">Delete<")
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_domain_info-0-portfolio")
+        self.assertContains(response, "id_domain_info-0-sub_organization")
+        self.assertNotContains(response, "id_domain_info-0-creator")
+        # self.assertNotContains(response, "id_email")
+        # self.assertContains(response, "closelink")
+        # self.assertNotContains(response, "Save")
+        # self.assertNotContains(response, "Delete")
+    
     @less_console_noise_decorator
     def test_staff_can_see_cisa_region_federal(self):
         """Tests if staff can see CISA Region: N/A"""

From bb913a937248c77c3dc39e54a683db5863b26c3e Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sun, 9 Mar 2025 19:21:09 -0400
Subject: [PATCH 28/47] fixed some fields that should have been readonly in
 Domains and Domain Requests

---
 src/registrar/admin.py                   | 61 +++++++++++++---
 src/registrar/tests/test_admin_domain.py | 91 ++++++++++++++++++++++--
 2 files changed, 136 insertions(+), 16 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 532b0b615..59d46eb55 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -411,17 +411,6 @@ class DomainInformationAdminForm(forms.ModelForm):
 
 class DomainInformationInlineForm(forms.ModelForm):
     """This form utilizes the custom widget for its class's ManyToMany UIs."""
-
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
-        # for OMB analysts, limit portfolio dropdown to FEB portfolios
-        user = self.request.user if hasattr(self, 'request') else None
-        if user and user.groups.filter(name="omb_analysts_group").exists():
-            self.fields["portfolio"].queryset = models.Portfolio.objects.filter(
-                Q(organization_type=DomainRequest.OrganizationChoices.FEDERAL) &
-                Q(federal_agency__federal_type=BranchChoices.EXECUTIVE)
-            )
     
     class Meta:
         model = models.DomainInformation
@@ -2343,6 +2332,47 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         "is_policy_acknowledged",
     ]
 
+    # Read only that we'll leverage for OMB Analysts
+    omb_analyst_readonly_fields = [
+        "federal_agency",
+        "creator",
+        "about_your_organization",
+        "anything_else",
+        "cisa_representative_first_name",
+        "cisa_representative_last_name",
+        "cisa_representative_email",
+        "domain_request",
+        "notes",
+        "senior_official",
+        "organization_type",
+        "organization_name",
+        "state_territory",
+        "address_line1",
+        "address_line2",
+        "city",
+        "zipcode",
+        "urbanization",
+        "portfolio_organization_type",
+        "portfolio_federal_type",
+        "portfolio_organization_name",
+        "portfolio_federal_agency",
+        "portfolio_state_territory",
+        "portfolio_address_line1",
+        "portfolio_address_line2",
+        "portfolio_city",
+        "portfolio_zipcode",
+        "portfolio_urbanization",
+        "organization_type",
+        "federal_type",
+        "federal_agency",
+        "tribe_name",
+        "federally_recognized_tribe",
+        "state_recognized_tribe",
+        "about_your_organization",
+        "portfolio",
+        "sub_organization",
+    ]
+
     # For each filter_horizontal, init in admin js initFilterHorizontalWidget
     # to activate the edit/delete/view buttons
     filter_horizontal = ("other_contacts",)
@@ -2371,6 +2401,10 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
 
         if request.user.has_perm("registrar.full_access_permission"):
             return readonly_fields
+        # Return restrictive Read-only fields for OMB analysts
+        if request.user.groups.filter(name="omb_analysts_group").exists():
+            readonly_fields.extend([field for field in self.omb_analyst_readonly_fields])
+            return readonly_fields
         # Return restrictive Read-only fields for analysts and
         # users who might not belong to groups
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
@@ -2992,6 +3026,10 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         "action_needed_reason",
         "action_needed_reason_email",
         "portfolio",
+        "sub_organization",
+        "requested_suborganization",
+        "suborganization_city",
+        "suborganization_state_territory",
     ]
 
     autocomplete_fields = [
@@ -3619,6 +3657,7 @@ class DomainInformationInline(admin.StackedInline):
     fieldsets = copy.deepcopy(list(DomainInformationAdmin.fieldsets))
     readonly_fields = copy.deepcopy(DomainInformationAdmin.readonly_fields)
     analyst_readonly_fields = copy.deepcopy(DomainInformationAdmin.analyst_readonly_fields)
+    omb_analyst_readonly_fields = copy.deepcopy(DomainInformationAdmin.omb_analyst_readonly_fields)
     autocomplete_fields = copy.deepcopy(DomainInformationAdmin.autocomplete_fields)
 
     def get_domain_managers(self, obj):
diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py
index 2122c576f..6a65f5f5e 100644
--- a/src/registrar/tests/test_admin_domain.py
+++ b/src/registrar/tests/test_admin_domain.py
@@ -51,6 +51,7 @@ class TestDomainAdminAsStaff(MockEppLib):
     @classmethod
     def setUpClass(self):
         super().setUpClass()
+        self.superuser = create_superuser()
         self.staffuser = create_user()
         self.omb_analyst = create_omb_analyst_user()
         self.site = AdminSite()
@@ -127,13 +128,93 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertContains(response, "Save")
         self.assertNotContains(response, ">Delete<")
         # test whether fields are readonly or editable
+        self.assertNotContains(response, "id_domain_info-0-portfolio")
+        self.assertNotContains(response, "id_domain_info-0-sub_organization")
+        self.assertNotContains(response, "id_domain_info-0-creator")
+        self.assertNotContains(response, "id_domain_info-0-federal_agency")
+        self.assertNotContains(response, "id_domain_info-0-about_your_organization")
+        self.assertNotContains(response, "id_domain_info-0-anything_else")
+        self.assertNotContains(response, "id_domain_info-0-cisa_representative_first_name")
+        self.assertNotContains(response, "id_domain_info-0-cisa_representative_last_name")
+        self.assertNotContains(response, "id_domain_info-0-cisa_representative_email")
+        self.assertNotContains(response, "id_domain_info-0-domain_request")
+        self.assertNotContains(response, "id_domain_info-0-notes")
+        self.assertNotContains(response, "id_domain_info-0-senior_official")
+        self.assertNotContains(response, "id_domain_info-0-organization_type")
+        self.assertNotContains(response, "id_domain_info-0-state_territory")
+        self.assertNotContains(response, "id_domain_info-0-address_line1")        
+        self.assertNotContains(response, "id_domain_info-0-address_line2")
+        self.assertNotContains(response, "id_domain_info-0-city")        
+        self.assertNotContains(response, "id_domain_info-0-zipcode")
+        self.assertNotContains(response, "id_domain_info-0-urbanization")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_organization_type")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_federal_type")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_organization_name")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_federal_agency")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_state_territory")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_address_line1")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_address_line2")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_city")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_zipcode")
+        self.assertNotContains(response, "id_domain_info-0-portfolio_urbanization")
+        self.assertNotContains(response, "id_domain_info-0-organization_type")
+        self.assertNotContains(response, "id_domain_info-0-federal_type")
+        self.assertNotContains(response, "id_domain_info-0-federal_agency")
+        self.assertNotContains(response, "id_domain_info-0-tribe_name")
+        self.assertNotContains(response, "id_domain_info-0-federally_recognized_tribe")
+        self.assertNotContains(response, "id_domain_info-0-state_recognized_tribe")
+        self.assertNotContains(response, "id_domain_info-0-about_your_organization")
+        self.assertNotContains(response, "id_domain_info-0-portfolio")
+        self.assertNotContains(response, "id_domain_info-0-sub_organization")
+    
+    @less_console_noise_decorator
+    def test_superuser_change(self):
+        """Ensure super user can view/edit all domains."""
+        self.client.force_login(self.superuser)
+        response = self.client.get(reverse("admin:registrar_domain_change", args=[self.nonfebdomain.id]))
+        self.assertEqual(response.status_code, 200)
+        response = self.client.get(reverse("admin:registrar_domain_change", args=[self.febdomain.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, self.febdomain.name)
+        # test portfolio dropdown
+        self.assertContains(response, self.portfolio.organization_name)
+        # test buttons
+        self.assertContains(response, "Manage domain")
+        self.assertContains(response, "Get registry status")
+        self.assertContains(response, "Extend expiration date")
+        self.assertContains(response, "Remove from registry")
+        self.assertContains(response, "Place hold")
+        self.assertContains(response, "Save")
+        self.assertContains(response, ">Delete<")
+        # test whether fields are readonly or editable
+        self.assertContains(response, "id_domain_info-0-portfolio")
+        self.assertContains(response, "id_domain_info-0-sub_organization")
+        self.assertContains(response, "id_domain_info-0-creator")
+        self.assertContains(response, "id_domain_info-0-federal_agency")
+        self.assertContains(response, "id_domain_info-0-about_your_organization")
+        self.assertContains(response, "id_domain_info-0-anything_else")
+        self.assertContains(response, "id_domain_info-0-cisa_representative_first_name")
+        self.assertContains(response, "id_domain_info-0-cisa_representative_last_name")
+        self.assertContains(response, "id_domain_info-0-cisa_representative_email")
+        self.assertContains(response, "id_domain_info-0-domain_request")
+        self.assertContains(response, "id_domain_info-0-notes")
+        self.assertContains(response, "id_domain_info-0-senior_official")
+        self.assertContains(response, "id_domain_info-0-organization_type")
+        self.assertContains(response, "id_domain_info-0-state_territory")
+        self.assertContains(response, "id_domain_info-0-address_line1")        
+        self.assertContains(response, "id_domain_info-0-address_line2")
+        self.assertContains(response, "id_domain_info-0-city")        
+        self.assertContains(response, "id_domain_info-0-zipcode")
+        self.assertContains(response, "id_domain_info-0-urbanization")
+        self.assertContains(response, "id_domain_info-0-organization_type")
+        self.assertContains(response, "id_domain_info-0-federal_type")
+        self.assertContains(response, "id_domain_info-0-federal_agency")
+        self.assertContains(response, "id_domain_info-0-tribe_name")
+        self.assertContains(response, "id_domain_info-0-federally_recognized_tribe")
+        self.assertContains(response, "id_domain_info-0-state_recognized_tribe")
+        self.assertContains(response, "id_domain_info-0-about_your_organization")
         self.assertContains(response, "id_domain_info-0-portfolio")
         self.assertContains(response, "id_domain_info-0-sub_organization")
-        self.assertNotContains(response, "id_domain_info-0-creator")
-        # self.assertNotContains(response, "id_email")
-        # self.assertContains(response, "closelink")
-        # self.assertNotContains(response, "Save")
-        # self.assertNotContains(response, "Delete")
     
     @less_console_noise_decorator
     def test_staff_can_see_cisa_region_federal(self):

From 8aeeb3c9e35c8d98e6029caf18908ffb832433f7 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Sun, 9 Mar 2025 19:27:43 -0400
Subject: [PATCH 29/47] lint

---
 src/registrar/admin.py                   |  4 ++--
 src/registrar/tests/test_admin_domain.py | 12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 59d46eb55..a5ab53071 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -411,7 +411,7 @@ class DomainInformationAdminForm(forms.ModelForm):
 
 class DomainInformationInlineForm(forms.ModelForm):
     """This form utilizes the custom widget for its class's ManyToMany UIs."""
-    
+
     class Meta:
         model = models.DomainInformation
         fields = "__all__"
@@ -3804,7 +3804,7 @@ class DomainInformationInline(admin.StackedInline):
         form.is_omb_analyst = self.is_omb_analyst
 
         return form
-    
+
     def get_formset(self, request, obj=None, **kwargs):
         """Attach request to the formset so that it can be available in the form"""
         formset = super().get_formset(request, obj, **kwargs)
diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py
index 6a65f5f5e..ab32f7c2a 100644
--- a/src/registrar/tests/test_admin_domain.py
+++ b/src/registrar/tests/test_admin_domain.py
@@ -142,9 +142,9 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertNotContains(response, "id_domain_info-0-senior_official")
         self.assertNotContains(response, "id_domain_info-0-organization_type")
         self.assertNotContains(response, "id_domain_info-0-state_territory")
-        self.assertNotContains(response, "id_domain_info-0-address_line1")        
+        self.assertNotContains(response, "id_domain_info-0-address_line1")
         self.assertNotContains(response, "id_domain_info-0-address_line2")
-        self.assertNotContains(response, "id_domain_info-0-city")        
+        self.assertNotContains(response, "id_domain_info-0-city")
         self.assertNotContains(response, "id_domain_info-0-zipcode")
         self.assertNotContains(response, "id_domain_info-0-urbanization")
         self.assertNotContains(response, "id_domain_info-0-portfolio_organization_type")
@@ -166,7 +166,7 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertNotContains(response, "id_domain_info-0-about_your_organization")
         self.assertNotContains(response, "id_domain_info-0-portfolio")
         self.assertNotContains(response, "id_domain_info-0-sub_organization")
-    
+
     @less_console_noise_decorator
     def test_superuser_change(self):
         """Ensure super user can view/edit all domains."""
@@ -201,9 +201,9 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertContains(response, "id_domain_info-0-senior_official")
         self.assertContains(response, "id_domain_info-0-organization_type")
         self.assertContains(response, "id_domain_info-0-state_territory")
-        self.assertContains(response, "id_domain_info-0-address_line1")        
+        self.assertContains(response, "id_domain_info-0-address_line1")
         self.assertContains(response, "id_domain_info-0-address_line2")
-        self.assertContains(response, "id_domain_info-0-city")        
+        self.assertContains(response, "id_domain_info-0-city")
         self.assertContains(response, "id_domain_info-0-zipcode")
         self.assertContains(response, "id_domain_info-0-urbanization")
         self.assertContains(response, "id_domain_info-0-organization_type")
@@ -215,7 +215,7 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertContains(response, "id_domain_info-0-about_your_organization")
         self.assertContains(response, "id_domain_info-0-portfolio")
         self.assertContains(response, "id_domain_info-0-sub_organization")
-    
+
     @less_console_noise_decorator
     def test_staff_can_see_cisa_region_federal(self):
         """Tests if staff can see CISA Region: N/A"""

From 64de8302545dc6dbb2ea6ccab4384cb0d26e8e28 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 10 Mar 2025 06:02:15 -0400
Subject: [PATCH 30/47] update tests for admin domain requests

---
 src/registrar/tests/test_admin_request.py | 156 ++++++++++++++++++++++
 1 file changed, 156 insertions(+)

diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py
index 968de0d65..e1e6f45d3 100644
--- a/src/registrar/tests/test_admin_request.py
+++ b/src/registrar/tests/test_admin_request.py
@@ -1,6 +1,8 @@
 from datetime import datetime
 from django.forms import ValidationError
 from django.utils import timezone
+from registrar.models.federal_agency import FederalAgency
+from registrar.utility.constants import BranchChoices
 from waffle.testutils import override_flag
 import re
 from django.test import RequestFactory, Client, TestCase, override_settings
@@ -37,6 +39,7 @@ from .common import (
     less_console_noise,
     create_superuser,
     create_user,
+    create_omb_analyst_user,
     multiple_unalphabetical_domain_objects,
     MockEppLib,
     GenericTestHelper,
@@ -68,6 +71,7 @@ class TestDomainRequestAdmin(MockEppLib):
         self.admin = DomainRequestAdmin(model=DomainRequest, admin_site=self.site)
         self.superuser = create_superuser()
         self.staffuser = create_user()
+        self.ombanalyst = create_omb_analyst_user()
         self.client = Client(HTTP_HOST="localhost:8080")
         self.test_helper = GenericTestHelper(
             factory=self.factory,
@@ -80,6 +84,12 @@ class TestDomainRequestAdmin(MockEppLib):
         allowed_emails = [AllowedEmail(email="mayor@igorville.gov"), AllowedEmail(email="help@get.gov")]
         AllowedEmail.objects.bulk_create(allowed_emails)
 
+    def setUp(self):
+        super().setUp()
+        self.fed_agency = FederalAgency.objects.create(
+            agency="New FedExec Agency", federal_type=BranchChoices.EXECUTIVE
+        )
+
     def tearDown(self):
         super().tearDown()
         Host.objects.all().delete()
@@ -92,6 +102,7 @@ class TestDomainRequestAdmin(MockEppLib):
         SeniorOfficial.objects.all().delete()
         Suborganization.objects.all().delete()
         Portfolio.objects.all().delete()
+        self.fed_agency.delete()
         self.mock_client.EMAILS_SENT.clear()
 
     @classmethod
@@ -100,6 +111,71 @@ class TestDomainRequestAdmin(MockEppLib):
         User.objects.all().delete()
         AllowedEmail.objects.all().delete()
 
+    @override_flag("organization_feature", active=True)
+    @less_console_noise_decorator
+    def test_omb_analyst_view(self):
+        """Ensure OMB analysts can view domain request list."""
+        febportfolio = Portfolio.objects.create(
+            organization_name="new portfolio",
+            organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+            federal_agency=self.fed_agency,
+            creator=self.ombanalyst,
+        )
+        nonfebportfolio = Portfolio.objects.create(
+            organization_name="non feb portfolio",
+            creator=self.ombanalyst,
+        )
+        nonfebdomainrequest = completed_domain_request(
+            name="test1234nonfeb.gov",
+            portfolio=nonfebportfolio,
+            status=DomainRequest.DomainRequestStatus.SUBMITTED,
+        )
+        febdomainrequest = completed_domain_request(
+            name="test1234feb.gov",
+            portfolio=febportfolio,
+            status=DomainRequest.DomainRequestStatus.SUBMITTED,
+        )
+        self.client.force_login(self.ombanalyst)
+        response = self.client.get(reverse("admin:registrar_domainrequest_changelist"))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, febdomainrequest.requested_domain.name)
+        self.assertNotContains(response, nonfebdomainrequest.requested_domain.name)
+        self.assertNotContains(response, ">Import<")
+        self.assertNotContains(response, ">Export<")
+
+    @less_console_noise_decorator
+    def test_omb_analyst_change(self):
+        """Ensure OMB analysts can view/edit federal executive branch domain requests."""
+        self.client.force_login(self.ombanalyst)
+        febportfolio = Portfolio.objects.create(
+            organization_name="new portfolio",
+            organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+            federal_agency=self.fed_agency,
+            creator=self.ombanalyst,
+        )
+        nonfebportfolio = Portfolio.objects.create(
+            organization_name="non feb portfolio",
+            creator=self.ombanalyst,
+        )
+        nonfebdomainrequest = completed_domain_request(
+            name="test1234nonfeb.gov",
+            portfolio=nonfebportfolio,
+            status=DomainRequest.DomainRequestStatus.SUBMITTED,
+        )
+        febdomainrequest = completed_domain_request(
+            name="test1234feb.gov",
+            portfolio=febportfolio,
+            status=DomainRequest.DomainRequestStatus.SUBMITTED,
+        )
+        response = self.client.get(reverse("admin:registrar_domainrequest_change", args=[nonfebdomainrequest.id]))
+        self.assertEqual(response.status_code, 302)
+        response = self.client.get(reverse("admin:registrar_domainrequest_change", args=[febdomainrequest.id]))
+        self.assertEqual(response.status_code, 200)
+        self.assertContains(response, febdomainrequest.requested_domain.name)
+        # test buttons
+        self.assertContains(response, "Save")
+        self.assertNotContains(response, ">Delete<")
+
     @override_flag("organization_feature", active=True)
     @less_console_noise_decorator
     def test_clean_validates_duplicate_suborganization(self):
@@ -2065,6 +2141,86 @@ class TestDomainRequestAdmin(MockEppLib):
 
             self.assertEqual(readonly_fields, expected_fields)
 
+    def test_readonly_fields_for_omb_analyst(self):
+        with less_console_noise():
+            request = self.factory.get("/")  # Use the correct method and path
+            request.user = self.ombanalyst
+
+            readonly_fields = self.admin.get_readonly_fields(request)
+
+            expected_fields = [
+                "portfolio_senior_official",
+                "portfolio_organization_type",
+                "portfolio_federal_type",
+                "portfolio_organization_name",
+                "portfolio_federal_agency",
+                "portfolio_state_territory",
+                "portfolio_address_line1",
+                "portfolio_address_line2",
+                "portfolio_city",
+                "portfolio_zipcode",
+                "portfolio_urbanization",
+                "other_contacts",
+                "current_websites",
+                "alternative_domains",
+                "is_election_board",
+                "status_history",
+                "federal_agency",
+                "creator",
+                "about_your_organization",
+                "requested_domain",
+                "approved_domain",
+                "alternative_domains",
+                "purpose",
+                "no_other_contacts_rationale",
+                "anything_else",
+                "is_policy_acknowledged",
+                "cisa_representative_first_name",
+                "cisa_representative_last_name",
+                "cisa_representative_email",
+                "status",
+                "investigator",
+                "notes",
+                "senior_official",
+                "organization_type",
+                "organization_name",
+                "state_territory",
+                "address_line1",
+                "address_line2",
+                "city",
+                "zipcode",
+                "urbanization",
+                "portfolio_organization_type",
+                "portfolio_federal_type",
+                "portfolio_organization_name",
+                "portfolio_federal_agency",
+                "portfolio_state_territory",
+                "portfolio_address_line1",
+                "portfolio_address_line2",
+                "portfolio_city",
+                "portfolio_zipcode",
+                "portfolio_urbanization",
+                "is_election_board",
+                "organization_type",
+                "federal_type",
+                "federal_agency",
+                "tribe_name",
+                "federally_recognized_tribe",
+                "state_recognized_tribe",
+                "about_your_organization",
+                "rejection_reason",
+                "rejection_reason_email",
+                "action_needed_reason",
+                "action_needed_reason_email",
+                "portfolio",
+                "sub_organization",
+                "requested_suborganization",
+                "suborganization_city",
+                "suborganization_state_territory",
+            ]
+
+            self.assertEqual(readonly_fields, expected_fields)
+
     def test_saving_when_restricted_creator(self):
         with less_console_noise():
             # Create an instance of the model

From d6746af538dbbd3c2bbba66688929223b16c6cd9 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 10 Mar 2025 06:09:15 -0400
Subject: [PATCH 31/47] fix test on Import

---
 src/registrar/tests/test_admin_domain.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/registrar/tests/test_admin_domain.py b/src/registrar/tests/test_admin_domain.py
index ab32f7c2a..aa6e799bd 100644
--- a/src/registrar/tests/test_admin_domain.py
+++ b/src/registrar/tests/test_admin_domain.py
@@ -104,8 +104,8 @@ class TestDomainAdminAsStaff(MockEppLib):
         self.assertEqual(response.status_code, 200)
         self.assertContains(response, self.febdomain.name)
         self.assertNotContains(response, self.nonfebdomain.name)
-        self.assertNotContains(response, "Import")
-        self.assertNotContains(response, "Export")
+        self.assertNotContains(response, ">Import<")
+        self.assertNotContains(response, ">Export<")
 
     @less_console_noise_decorator
     def test_omb_analyst_change(self):

From 15c41cdce4ef35a8de0263d06e99c508647ef376 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 10 Mar 2025 06:18:55 -0400
Subject: [PATCH 32/47] removed federal agency delete for OMB analysts

---
 src/registrar/models/user_group.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index 781dbb64c..f2ffa50ed 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -171,7 +171,7 @@ class UserGroup(Group):
             {
                 "app_label": "registrar",
                 "model": "federalagency",
-                "permissions": ["change_federalagency", "delete_federalagency"],
+                "permissions": ["change_federalagency"],
             },
             {
                 "app_label": "registrar",

From b5ab09db823a2aecea804f8b3485a6e57a886978 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 10 Mar 2025 06:24:10 -0400
Subject: [PATCH 33/47] removed federal agency delete for OMB analysts

---
 src/registrar/admin.py            | 9 ---------
 src/registrar/tests/test_admin.py | 2 +-
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index a5ab53071..9bb9efe69 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -5025,15 +5025,6 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_change_permission(request, obj)
 
-    def has_delete_permission(self, request, obj=None):
-        """Restrict delete permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_delete_permission(request, obj)
-
     def get_readonly_fields(self, request, obj=None):
         """Set the read-only state on form elements.
         We have 2 conditions that determine which fields are read-only:
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index e89a6352c..7e34c63c7 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3821,7 +3821,7 @@ class TestFederalAgencyAdmin(TestCase):
         self.assertNotContains(response, "id_is_fceb")
         self.assertNotContains(response, "closelink")
         self.assertContains(response, "Save")
-        self.assertContains(response, "Delete")
+        self.assertNotContains(response, "Delete")
 
     @less_console_noise_decorator
     def test_superuser_change(self):

From 08d6b70cfe39e93e3be823613fe811c5d90effbe Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 13 Mar 2025 22:02:36 -0400
Subject: [PATCH 34/47] added comments and removed commented out code

---
 .../src/js/getgov-admin/domain-request-form.js       | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
index 8823cb46c..b1c6feb04 100644
--- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
@@ -105,8 +105,8 @@ export function initApprovedDomain() {
             return;
         }
 
-        const statusToCheck = "approved";
-        const readonlyStatusToCheck = "Approved";
+        const statusToCheck = "approved";  // when checking against a select
+        const readonlyStatusToCheck = "Approved";  // when checking against a readonly div display value
         const statusSelect = document.getElementById("id_status");
         const statusField = document.querySelector("field-status");
         const sessionVariableName = "showApprovedDomain";
@@ -122,6 +122,8 @@ export function initApprovedDomain() {
 
         // Handle showing/hiding the related fields on page load.
         function initializeFormGroups() {
+            // Status is either in a select or in a readonly div. Both
+            // cases are handled below.
             let isStatus = false;
             if (statusSelect) {
                 isStatus = statusSelect.value == statusToCheck;
@@ -605,12 +607,6 @@ export function initActionNeededEmail() {
         // Initialize UI
         const customEmail = new customActionNeededEmail();
 
-        // // Check that every variable was setup correctly
-        // const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
-        // if (nullItems.length > 0) {
-        //     console.error(`Failed to load customActionNeededEmail(). Some variables were null: ${nullItems.join(", ")}`)
-        //     return;
-        // }
         customEmail.loadActionNeededEmail()
     });
 }

From f5429b97b0e2954de640aefa8e7f5668680acdde Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 13 Mar 2025 22:04:42 -0400
Subject: [PATCH 35/47] removed commented out code

---
 .../assets/src/js/getgov-admin/domain-request-form.js       | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
index b1c6feb04..e7bf8f00e 100644
--- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
@@ -669,12 +669,6 @@ export function initRejectedEmail() {
 
         // Initialize UI
         const customEmail = new customRejectedEmail();
-        // // Check that every variable was setup correctly
-        // const nullItems = Object.entries(customEmail.config).filter(([key, value]) => value === null).map(([key]) => key);
-        // if (nullItems.length > 0) {
-        //     console.error(`Failed to load customRejectedEmail(). Some variables were null: ${nullItems.join(", ")}`)
-        //     return;
-        // }
         customEmail.loadRejectedEmail()
     });
 }

From e15499e4a60b410de62b7f6d7d981228daa5bfcf Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 17 Mar 2025 13:20:14 -0400
Subject: [PATCH 36/47] aligned logic on federal type across the application

---
 src/registrar/admin.py                     | 18 ++++++++--------
 src/registrar/models/domain_information.py |  4 +++-
 src/registrar/models/domain_request.py     |  4 +++-
 src/registrar/tests/test_reports.py        | 24 +++++++++++-----------
 src/registrar/utility/csv_export.py        |  8 ++++----
 5 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 9bb9efe69..ac069bb38 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1793,7 +1793,7 @@ class DomainInvitationAdmin(BaseInvitationAdmin):
                     & Q(domain__domain_info__portfolio__federal_agency__isnull=False),
                     then=F("domain__domain_info__portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return the federal agency's federal_type
                 default=F("domain__domain_info__federal_agency__federal_type"),
             ),
         )
@@ -2435,7 +2435,7 @@ class DomainInformationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                     Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
                     then=F("portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return the federal_type from federal agency
                 default=F("federal_agency__federal_type"),
             ),
         )
@@ -2520,7 +2520,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
     class FederalTypeFilter(admin.SimpleListFilter):
         """Custom Federal Type filter that accomodates portfolio feature.
         If we have a portfolio, use the portfolio's federal type.  If not, use the
-        organization in the Domain Request object."""
+        organization in the Domain Request object's federal agency."""
 
         title = "federal type"
         parameter_name = "converted_federal_types"
@@ -2561,7 +2561,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             if self.value():
                 return queryset.filter(
                     Q(portfolio__federal_agency__federal_type=self.value())
-                    | Q(portfolio__isnull=True, federal_type=self.value())
+                    | Q(portfolio__isnull=True, federal_agency__federal_type=self.value())
                 )
             return queryset
 
@@ -3474,7 +3474,7 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                     Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
                     then=F("portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return federal type from federal agency
                 default=F("federal_agency__federal_type"),
             ),
         )
@@ -3932,7 +3932,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             if self.value():
                 return queryset.filter(
                     Q(domain_info__portfolio__federal_type=self.value())
-                    | Q(domain_info__portfolio__isnull=True, domain_info__federal_type=self.value())
+                    | Q(domain_info__portfolio__isnull=True, domain_info__federal_agency__federal_type=self.value())
                 )
             return queryset
 
@@ -3959,7 +3959,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                     Q(domain_info__portfolio__isnull=False) & Q(domain_info__portfolio__federal_agency__isnull=False),
                     then=F("domain_info__portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return federal type from federal agency
                 default=F("domain_info__federal_agency__federal_type"),
             ),
             converted_organization_name=Case(
@@ -4872,7 +4872,7 @@ class PortfolioAdmin(ListHeaderAdmin):
                     Q(federal_agency__isnull=False),
                     then=F("federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return empty string
                 default=Value(""),
             ),
         )
@@ -5164,7 +5164,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                     Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
                     then=F("portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
+                # Otherwise, return empty string
                 default=Value(""),
             ),
         )
diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py
index aa933e282..3839e5290 100644
--- a/src/registrar/models/domain_information.py
+++ b/src/registrar/models/domain_information.py
@@ -449,7 +449,9 @@ class DomainInformation(TimeStampedModel):
     def converted_federal_type(self):
         if self.portfolio:
             return self.portfolio.federal_type
-        return self.federal_type
+        elif self.federal_agency:
+            return self.federal_agency.federal_type
+        return None
 
     @property
     def converted_senior_official(self):
diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py
index 1cca3742f..66519e9f0 100644
--- a/src/registrar/models/domain_request.py
+++ b/src/registrar/models/domain_request.py
@@ -1454,7 +1454,9 @@ class DomainRequest(TimeStampedModel):
     def converted_federal_type(self):
         if self.portfolio:
             return self.portfolio.federal_type
-        return self.federal_type
+        elif self.federal_agency:
+            return self.federal_agency.federal_type
+        return None
 
     @property
     def converted_address_line1(self):
diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py
index 9ec3bd0d3..236e810cf 100644
--- a/src/registrar/tests/test_reports.py
+++ b/src/registrar/tests/test_reports.py
@@ -72,7 +72,7 @@ class CsvReportsTest(MockDbForSharedTests):
             fake_open = mock_open()
             expected_file_content = [
                 call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"),
-                call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"),
+                call("cdomain11.gov,Federal,World War I Centennial Commission,,,,(blank)\r\n"),
                 call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"),
                 call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"),
                 call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"),
@@ -94,7 +94,7 @@ class CsvReportsTest(MockDbForSharedTests):
             fake_open = mock_open()
             expected_file_content = [
                 call("Domain name,Domain type,Agency,Organization name,City,State,Security contact email\r\n"),
-                call("cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"),
+                call("cdomain11.gov,Federal,World War I Centennial Commission,,,,(blank)\r\n"),
                 call("cdomain1.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\r\n"),
                 call("adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"),
                 call("ddomain3.gov,Federal,Armed Forces Retirement Home,,,,(blank)\r\n"),
@@ -261,9 +261,6 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             "defaultsecurity.gov,Ready,2023-11-01,(blank),Federal - Executive,"
             "Portfolio 1 Federal Agency,Portfolio 1 Federal Agency,,, ,,(blank),"
             '"big_lebowski@dude.co, info@example.com, meoward@rocks.com",woofwardthethird@rocks.com\n'
-            "cdomain11.gov,Ready,2024-04-02,(blank),Federal - Executive,"
-            "World War I Centennial Commission,,,, ,,(blank),"
-            "meoward@rocks.com,\n"
             "adomain10.gov,Ready,2024-04-03,(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,"
             "squeaker@rocks.com\n"
             "bdomain4.gov,Unknown,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n"
@@ -274,6 +271,9 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             "sdomain8.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n"
             "xdomain7.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n"
             "zdomain9.gov,Deleted,(blank),(blank),Federal,Armed Forces Retirement Home,,,, ,,(blank),,\n"
+            "cdomain11.gov,Ready,2024-04-02,(blank),Federal,"
+            "World War I Centennial Commission,,,, ,,(blank),"
+            "meoward@rocks.com,\n"
             "zdomain12.gov,Ready,2024-04-02,(blank),Interstate,,,,, ,,(blank),meoward@rocks.com,\n"
         )
 
@@ -498,7 +498,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
         # sorted alphabetially by domain name
         expected_content = (
             "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n"
-            "cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n"
+            "cdomain11.gov,Federal,World War I Centennial Commission,,,,(blank)\n"
             "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n"
             "adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\n"
             "ddomain3.gov,Federal,Armed Forces Retirement Home,,,,security@mail.gov\n"
@@ -538,7 +538,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
         # sorted alphabetially by domain name
         expected_content = (
             "Domain name,Domain type,Agency,Organization name,City,State,Security contact email\n"
-            "cdomain11.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n"
+            "cdomain11.gov,Federal,World War I Centennial Commission,,,,(blank)\n"
             "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,,,,(blank)\n"
             "adomain10.gov,Federal,Armed Forces Retirement Home,,,,(blank)\n"
             "ddomain3.gov,Federal,Armed Forces Retirement Home,,,,security@mail.gov\n"
@@ -594,7 +594,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
                     "State,Status,Expiration date, Deleted\n"
                     "cdomain1.gov,Federal-Executive,Portfolio1FederalAgency,Portfolio1FederalAgency,Ready,(blank)\n"
                     "adomain10.gov,Federal,ArmedForcesRetirementHome,Ready,(blank)\n"
-                    "cdomain11.gov,Federal-Executive,WorldWarICentennialCommission,Ready,(blank)\n"
+                    "cdomain11.gov,Federal,WorldWarICentennialCommission,Ready,(blank)\n"
                     "zdomain12.gov,Interstate,Ready,(blank)\n"
                     "zdomain9.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-01\n"
                     "sdomain8.gov,Federal,ArmedForcesRetirementHome,Deleted,(blank),2024-04-02\n"
@@ -642,7 +642,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             "3,2,1,0,0,0,0,0,0,0\n"
             "\n"
             "Domain name,Domain type,Domain managers,Invited domain managers\n"
-            "cdomain11.gov,Federal - Executive,meoward@rocks.com,\n"
+            "cdomain11.gov,Federal,meoward@rocks.com,\n"
             'cdomain1.gov,Federal - Executive,"big_lebowski@dude.co, info@example.com, meoward@rocks.com",'
             "woofwardthethird@rocks.com\n"
             "zdomain12.gov,Interstate,meoward@rocks.com,\n"
@@ -716,7 +716,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
             expected_content = (
                 "Domain request,Domain type,Federal type\n"
                 "city3.gov,Federal,Executive\n"
-                "city4.gov,City,Executive\n"
+                "city4.gov,City,\n"
                 "city6.gov,Federal,Executive\n"
             )
 
@@ -783,7 +783,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
                 "SO last name,SO email,SO title/role,Request purpose,Request additional details,Other contacts,"
                 "CISA regional representative,Current websites,Investigator\n"
                 # Content
-                "city5.gov,Approved,Federal,No,Executive,,Testorg,N/A,,NY,2,requested_suborg,SanFran,CA,,,,,1,0,"
+                "city5.gov,Approved,Federal,No,,,Testorg,N/A,,NY,2,requested_suborg,SanFran,CA,,,,,1,0,"
                 "city1.gov,Testy,Tester,testy@town.com,Chief Tester,Purpose of the site,There is more,"
                 "Testy Tester testy2@town.com,,city.com,\n"
                 "city2.gov,In review,Federal,Yes,Executive,Portfolio 1 Federal Agency,Portfolio 1 Federal Agency,"
@@ -795,7 +795,7 @@ class ExportDataTest(MockDbForIndividualTests, MockEppLib):
                 'There is more,"Meow Tester24 te2@town.com, Testy1232 Tester24 te2@town.com, '
                 'Testy Tester testy2@town.com",'
                 'test@igorville.com,"city.com, https://www.example2.com, https://www.example.com",\n'
-                "city4.gov,Submitted,City,No,Executive,,Testorg,Yes,,NY,2,,,,,,,,0,1,city1.gov,Testy,"
+                "city4.gov,Submitted,City,No,,,Testorg,Yes,,NY,2,,,,,,,,0,1,city1.gov,Testy,"
                 "Tester,testy@town.com,"
                 "Chief Tester,Purpose of the site,CISA-first-name CISA-last-name | There is more,"
                 "Testy Tester testy2@town.com,"
diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py
index fad58b2e2..cde91baca 100644
--- a/src/registrar/utility/csv_export.py
+++ b/src/registrar/utility/csv_export.py
@@ -579,8 +579,8 @@ class DomainExport(BaseExport):
                     Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
                     then=F("portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
-                default=F("federal_type"),
+                # Otherwise, return the federal type from federal agency
+                default=F("federal_agency__federal_type"),
                 output_field=CharField(),
             ),
             "converted_organization_name": Case(
@@ -1654,8 +1654,8 @@ class DomainRequestExport(BaseExport):
                     Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
                     then=F("portfolio__federal_agency__federal_type"),
                 ),
-                # Otherwise, return the natively assigned value
-                default=F("federal_type"),
+                # Otherwise, return the federal type from federal agency
+                default=F("federal_agency__federal_type"),
                 output_field=CharField(),
             ),
             "converted_organization_name": Case(

From dcfa9e4804396eeac8f0e8a5fdf087f249b79a92 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 17 Mar 2025 13:42:25 -0400
Subject: [PATCH 37/47] fixed migrations

---
 .../{0142_create_groups_v18.py => 0143_create_groups_v18.py}    | 2 +-
 .../{0143_alter_user_options.py => 0144_alter_user_options.py}  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename src/registrar/migrations/{0142_create_groups_v18.py => 0143_create_groups_v18.py} (93%)
 rename src/registrar/migrations/{0143_alter_user_options.py => 0144_alter_user_options.py} (92%)

diff --git a/src/registrar/migrations/0142_create_groups_v18.py b/src/registrar/migrations/0143_create_groups_v18.py
similarity index 93%
rename from src/registrar/migrations/0142_create_groups_v18.py
rename to src/registrar/migrations/0143_create_groups_v18.py
index 74d4c2f50..d0b7a6dbc 100644
--- a/src/registrar/migrations/0142_create_groups_v18.py
+++ b/src/registrar/migrations/0143_create_groups_v18.py
@@ -26,7 +26,7 @@ def create_groups(apps, schema_editor) -> Any:
 
 class Migration(migrations.Migration):
     dependencies = [
-        ("registrar", "0141_alter_portfolioinvitation_additional_permissions_and_more"),
+        ("registrar", "0142_domainrequest_feb_naming_requirements_and_more"),
     ]
 
     operations = [
diff --git a/src/registrar/migrations/0143_alter_user_options.py b/src/registrar/migrations/0144_alter_user_options.py
similarity index 92%
rename from src/registrar/migrations/0143_alter_user_options.py
rename to src/registrar/migrations/0144_alter_user_options.py
index 58e5cf3d5..88b06afe8 100644
--- a/src/registrar/migrations/0143_alter_user_options.py
+++ b/src/registrar/migrations/0144_alter_user_options.py
@@ -6,7 +6,7 @@ from django.db import migrations
 class Migration(migrations.Migration):
 
     dependencies = [
-        ("registrar", "0142_create_groups_v18"),
+        ("registrar", "0143_create_groups_v18"),
     ]
 
     operations = [

From 9ff8e8e0fbe71675905796e4820dbf6be672ea6b Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 17 Mar 2025 14:56:50 -0400
Subject: [PATCH 38/47] removed save buttons on readonly tables

---
 src/registrar/models/user_group.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index f2ffa50ed..c70879943 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -171,22 +171,22 @@ class UserGroup(Group):
             {
                 "app_label": "registrar",
                 "model": "federalagency",
-                "permissions": ["change_federalagency"],
+                "permissions": ["view_federalagency"],
             },
             {
                 "app_label": "registrar",
                 "model": "portfolio",
-                "permissions": ["change_portfolio"],
+                "permissions": ["view_portfolio"],
             },
             {
                 "app_label": "registrar",
                 "model": "suborganization",
-                "permissions": ["change_suborganization"],
+                "permissions": ["view_suborganization"],
             },
             {
                 "app_label": "registrar",
                 "model": "seniorofficial",
-                "permissions": ["change_seniorofficial"],
+                "permissions": ["view_seniorofficial"],
             },
         ]
 

From f275245f698dad4922a6ac3b6314b6f819b7bcee Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 17 Mar 2025 15:06:49 -0400
Subject: [PATCH 39/47] removed save buttons on readonly tables

---
 src/registrar/admin.py | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 72bf7beb6..dbf5aa0ae 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1473,15 +1473,6 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
                 return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
 
-    def has_change_permission(self, request, obj=None):
-        """Restrict update permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_agency and obj.federal_agency.federal_type == BranchChoices.EXECUTIVE
-        return super().has_change_permission(request, obj)
-
 
 class WebsiteResource(resources.ModelResource):
     """defines how each field in the referenced model should be mapped to the corresponding fields in the
@@ -4898,15 +4889,6 @@ class PortfolioAdmin(ListHeaderAdmin):
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
 
-    def has_change_permission(self, request, obj=None):
-        """Restrict update permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_change_permission(request, obj)
-
     def change_view(self, request, object_id, form_url="", extra_context=None):
         """Add related suborganizations and domain groups.
         Add the summary for the portfolio members field (list of members that link to change_forms)."""
@@ -5013,15 +4995,6 @@ class FederalAgencyAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 return obj.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
 
-    def has_change_permission(self, request, obj=None):
-        """Restrict update permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.federal_type == BranchChoices.EXECUTIVE
-        return super().has_change_permission(request, obj)
-
     def get_readonly_fields(self, request, obj=None):
         """Set the read-only state on form elements.
         We have 2 conditions that determine which fields are read-only:
@@ -5193,15 +5166,6 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
                 return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
 
-    def has_change_permission(self, request, obj=None):
-        """Restrict update permissions based on group membership and model attributes."""
-        if request.user.has_perm("registrar.full_access_permission"):
-            return True
-        if obj:
-            if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
-        return super().has_change_permission(request, obj)
-
 
 class AllowedEmailAdmin(ListHeaderAdmin):
     class Meta:

From 07b1010dd4b4534fc71f38096df47d2bfc86a4aa Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Mon, 17 Mar 2025 15:31:14 -0400
Subject: [PATCH 40/47] updated tests related to updated permissions

---
 src/registrar/admin.py            | 9 +++++----
 src/registrar/tests/test_admin.py | 8 ++++----
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index dbf5aa0ae..1df862255 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1457,10 +1457,11 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
 
         # Check if user is in OMB analysts group
         if request.user.groups.filter(name="omb_analysts_group").exists():
-            annotated_qs = self.get_annotated_queryset(qs)
-            return annotated_qs.filter(
-                converted_federal_type=BranchChoices.EXECUTIVE,
-            )
+            # annotated_qs = self.get_annotated_queryset(qs)
+            # return annotated_qs.filter(
+            #     converted_federal_type=BranchChoices.EXECUTIVE,
+            # )
+            return qs.filter(federal_agency__federal_type=BranchChoices.EXECUTIVE)
 
         return qs  # Return full queryset if the user doesn't have the restriction
 
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index 7e34c63c7..8fd2744ec 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -3819,8 +3819,8 @@ class TestFederalAgencyAdmin(TestCase):
         self.assertNotContains(response, "id_federal_type")
         self.assertNotContains(response, "id_acronym")
         self.assertNotContains(response, "id_is_fceb")
-        self.assertNotContains(response, "closelink")
-        self.assertContains(response, "Save")
+        self.assertContains(response, "closelink")
+        self.assertNotContains(response, "Save")
         self.assertNotContains(response, "Delete")
 
     @less_console_noise_decorator
@@ -4083,8 +4083,8 @@ class TestPortfolioAdmin(TestCase):
         self.assertNotContains(response, "id_city")
         self.assertNotContains(response, "id_zipcode")
         self.assertNotContains(response, "id_urbanization")
-        self.assertNotContains(response, "closelink")
-        self.assertContains(response, "Save")
+        self.assertContains(response, "closelink")
+        self.assertNotContains(response, "Save")
         self.assertNotContains(response, "Delete")
 
     @less_console_noise_decorator

From 192a7bb8a40989a77f70929b85123ff6b415cac1 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 06:50:49 -0400
Subject: [PATCH 41/47] removed unnecessary code from SeniorOfficialAdmin

---
 src/registrar/admin.py | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 1df862255..bb729f20e 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -1404,19 +1404,6 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
     # in autocomplete_fields for Senior Official
     ordering = ["first_name", "last_name"]
 
-    def get_annotated_queryset(self, queryset):
-        return queryset.annotate(
-            converted_federal_type=Case(
-                # When portfolio is present, use its value instead
-                When(
-                    Q(federal_agency__isnull=False),
-                    then=F("federal_agency__federal_type"),
-                ),
-                # Otherwise, return the natively assigned value
-                default=Value(""),
-            ),
-        )
-
     readonly_fields = []
 
     # Even though this is empty, I will leave it as a stub for easy changes in the future
@@ -1457,10 +1444,6 @@ class SeniorOfficialAdmin(ListHeaderAdmin):
 
         # Check if user is in OMB analysts group
         if request.user.groups.filter(name="omb_analysts_group").exists():
-            # annotated_qs = self.get_annotated_queryset(qs)
-            # return annotated_qs.filter(
-            #     converted_federal_type=BranchChoices.EXECUTIVE,
-            # )
             return qs.filter(federal_agency__federal_type=BranchChoices.EXECUTIVE)
 
         return qs  # Return full queryset if the user doesn't have the restriction

From 5528390ee9a83332f1097e74feec3384afc04063 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 07:45:42 -0400
Subject: [PATCH 42/47] cleaned up unnecessary code

---
 src/registrar/admin.py | 47 +++++-------------------------------------
 1 file changed, 5 insertions(+), 42 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index bb729f20e..b2d9e9e97 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -4836,19 +4836,6 @@ class PortfolioAdmin(ListHeaderAdmin):
         readonly_fields.extend([field for field in self.analyst_readonly_fields])
         return readonly_fields
 
-    def get_annotated_queryset(self, queryset):
-        return queryset.annotate(
-            converted_federal_type=Case(
-                # When portfolio is present, use its value instead
-                When(
-                    Q(federal_agency__isnull=False),
-                    then=F("federal_agency__federal_type"),
-                ),
-                # Otherwise, return empty string
-                default=Value(""),
-            ),
-        )
-
     def get_queryset(self, request):
         """Restrict queryset based on user permissions."""
         qs = super().get_queryset(request)
@@ -4856,11 +4843,7 @@ class PortfolioAdmin(ListHeaderAdmin):
         # Check if user is in OMB analysts group
         if request.user.groups.filter(name="omb_analysts_group").exists():
             self.is_omb_analyst = True
-            annotated_qs = self.get_annotated_queryset(qs)
-            return annotated_qs.filter(
-                organization_type=DomainRequest.OrganizationChoices.FEDERAL,
-                converted_federal_type=BranchChoices.EXECUTIVE,
-            )
+            return qs.filter(federal_agency__federal_type=BranchChoices.EXECUTIVE)
 
         return qs  # Return full queryset if the user doesn't have the restriction
 
@@ -5110,34 +5093,14 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         extra_context = {"domain_requests": domain_requests, "domains": domains}
         return super().change_view(request, object_id, form_url, extra_context)
 
-    def get_annotated_queryset(self, queryset):
-        return queryset.annotate(
-            converted_federal_type=Case(
-                # When portfolio is present, use its value instead
-                When(
-                    Q(portfolio__isnull=False) & Q(portfolio__federal_agency__isnull=False),
-                    then=F("portfolio__federal_agency__federal_type"),
-                ),
-                # Otherwise, return empty string
-                default=Value(""),
-            ),
-        )
-
     def get_queryset(self, request):
-        """Custom get_queryset to filter by portfolio if portfolio is in the
-        request params."""
+        """Custom get_queryset to filter for OMB analysts."""
         qs = super().get_queryset(request)
-        # Check if a 'portfolio' parameter is passed in the request
-        portfolio_id = request.GET.get("portfolio")
-        if portfolio_id:
-            # Further filter the queryset by the portfolio
-            qs = qs.filter(portfolio=portfolio_id)
         # Check if user is in OMB analysts group
         if request.user.groups.filter(name="omb_analysts_group").exists():
-            annotated_qs = self.get_annotated_queryset(qs)
-            return annotated_qs.filter(
+            return qs.filter(
                 portfolio__organization_type=DomainRequest.OrganizationChoices.FEDERAL,
-                converted_federal_type=BranchChoices.EXECUTIVE,
+                portfolio__federal_agency__federal_type=BranchChoices.EXECUTIVE,
             )
         return qs
 
@@ -5147,7 +5110,7 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.portfolio and obj.portfolio.federal_type == BranchChoices.EXECUTIVE
+                return obj.portfolio and obj.portfolio.federal_agency and obj.portfolio.federal_agency.federal_type == BranchChoices.EXECUTIVE
         return super().has_view_permission(request, obj)
 
 

From 531d8c7e9411076271dca9879d96fb040059d51f Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 09:02:14 -0400
Subject: [PATCH 43/47] removed unnecessary permission

---
 src/registrar/admin.py                        |  2 +-
 .../js/getgov-admin/domain-request-form.js    | 10 ++++----
 src/registrar/decorators.py                   |  2 +-
 .../migrations/0144_alter_user_options.py     | 23 -------------------
 src/registrar/models/user.py                  |  1 -
 src/registrar/models/user_group.py            |  5 ----
 6 files changed, 8 insertions(+), 35 deletions(-)
 delete mode 100644 src/registrar/migrations/0144_alter_user_options.py

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index b2d9e9e97..c2be81066 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -4361,7 +4361,7 @@ class DomainAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
         if (
             request.user.has_perm("registrar.full_access_permission")
             or request.user.has_perm("registrar.analyst_access_permission")
-            or request.user.has_perm("registrar.omb_analyst_access_permission")
+            or request.user.groups.filter(name="omb_analysts_group").exists()
         ):
             return True
         return super().has_change_permission(request, obj)
diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
index e7bf8f00e..16acaf91c 100644
--- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
@@ -465,23 +465,25 @@ class CustomizableEmailBase {
     }
 
     initializeModalConfirm() {
+        // When the modal confirm button is present, add a listener
         if (this.modalConfirm) {
             this.modalConfirm.addEventListener("click", () => {
                 this.textarea.removeAttribute('readonly');
                 this.textarea.focus();
-                    hideElement(this.directEditButton);
-                    hideElement(this.modalTrigger);  
+                hideElement(this.directEditButton);
+                hideElement(this.modalTrigger);  
             });
         }
     }
 
     initializeDirectEditButton() {
+        // When the direct edit button is present, add a listener
         if (this.directEditButton) {
             this.directEditButton.addEventListener("click", () => {
                 this.textarea.removeAttribute('readonly');
                 this.textarea.focus();
-                    hideElement(this.directEditButton);
-                    hideElement(this.modalTrigger);  
+                hideElement(this.directEditButton);
+                hideElement(this.modalTrigger);  
             });
         }
     }
diff --git a/src/registrar/decorators.py b/src/registrar/decorators.py
index fcff02b32..d607935a2 100644
--- a/src/registrar/decorators.py
+++ b/src/registrar/decorators.py
@@ -112,7 +112,7 @@ def _user_has_permission(user, request, rules, **kwargs):
     permission_checks = [
         (IS_STAFF, lambda: user.is_staff),
         (IS_CISA_ANALYST, lambda: user.has_perm("registrar.analyst_access_permission")),
-        (IS_OMB_ANALYST, lambda: user.has_perm("registrar.omb_analyst_access_permission")),
+        (IS_OMB_ANALYST, lambda: user.groups.filter(name="omb_analysts_group").exists()),
         (IS_FULL_ACCESS, lambda: user.has_perm("registrar.full_access_permission")),
         (
             IS_DOMAIN_MANAGER,
diff --git a/src/registrar/migrations/0144_alter_user_options.py b/src/registrar/migrations/0144_alter_user_options.py
deleted file mode 100644
index 88b06afe8..000000000
--- a/src/registrar/migrations/0144_alter_user_options.py
+++ /dev/null
@@ -1,23 +0,0 @@
-# Generated by Django 4.2.17 on 2025-03-06 20:00
-
-from django.db import migrations
-
-
-class Migration(migrations.Migration):
-
-    dependencies = [
-        ("registrar", "0143_create_groups_v18"),
-    ]
-
-    operations = [
-        migrations.AlterModelOptions(
-            name="user",
-            options={
-                "permissions": [
-                    ("analyst_access_permission", "Analyst Access Permission"),
-                    ("omb_analyst_access_permission", "OMB Analyst Access Permission"),
-                    ("full_access_permission", "Full Access Permission"),
-                ]
-            },
-        ),
-    ]
diff --git a/src/registrar/models/user.py b/src/registrar/models/user.py
index 7ee4fefdf..d5476ab9a 100644
--- a/src/registrar/models/user.py
+++ b/src/registrar/models/user.py
@@ -40,7 +40,6 @@ class User(AbstractUser):
 
         permissions = [
             ("analyst_access_permission", "Analyst Access Permission"),
-            ("omb_analyst_access_permission", "OMB Analyst Access Permission"),
             ("full_access_permission", "Full Access Permission"),
         ]
 
diff --git a/src/registrar/models/user_group.py b/src/registrar/models/user_group.py
index c70879943..331e36605 100644
--- a/src/registrar/models/user_group.py
+++ b/src/registrar/models/user_group.py
@@ -158,11 +158,6 @@ class UserGroup(Group):
                 "model": "domain",
                 "permissions": ["view_domain"],
             },
-            {
-                "app_label": "registrar",
-                "model": "user",
-                "permissions": ["omb_analyst_access_permission"],
-            },
             {
                 "app_label": "registrar",
                 "model": "domaininvitation",

From 153d92542b8a198b7ac65980c14e827b503a3ee2 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 09:14:52 -0400
Subject: [PATCH 44/47] linted

---
 src/registrar/admin.py | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index c2be81066..83c547269 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -5110,7 +5110,11 @@ class SuborganizationAdmin(ListHeaderAdmin, ImportExportRegistrarModelAdmin):
             return True
         if obj:
             if request.user.groups.filter(name="omb_analysts_group").exists():
-                return obj.portfolio and obj.portfolio.federal_agency and obj.portfolio.federal_agency.federal_type == BranchChoices.EXECUTIVE
+                return (
+                    obj.portfolio
+                    and obj.portfolio.federal_agency
+                    and obj.portfolio.federal_agency.federal_type == BranchChoices.EXECUTIVE
+                )
         return super().has_view_permission(request, obj)
 
 

From a37cef619bfb063513fd8f9ed8bffc3ed20f0ba6 Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 11:01:09 -0400
Subject: [PATCH 45/47] fixed dynamic javascript on domain change form when
 portfolio readonly

---
 .../helpers-portfolio-dynamic-fields.js       | 44 ++++++++++++++++---
 1 file changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
index 54d0e073b..3880e63fa 100644
--- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
+++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js
@@ -12,7 +12,9 @@ export function handlePortfolioSelection(
     suborgDropdownSelector="#id_sub_organization"
 ) {
     // These dropdown are select2 fields so they must be interacted with via jquery
+    // In the event that these fields are readonly, need a variable to reference their row
     const portfolioDropdown = django.jQuery(portfolioDropdownSelector);
+    const portfolioField = document.querySelector(".field-portfolio");
     const suborganizationDropdown = django.jQuery(suborgDropdownSelector);
     const suborganizationField = document.querySelector(".field-sub_organization");
     const requestedSuborganizationField = document.querySelector(".field-requested_suborganization");
@@ -394,14 +396,30 @@ export function handlePortfolioSelection(
      * - Various global field elements (e.g., `suborganizationField`, `seniorOfficialField`, `portfolioOrgTypeFieldSet`) are used.
      */
     function updatePortfolioFieldsDisplay() {
-        // Retrieve the selected portfolio ID
-        let portfolio_id = portfolioDropdown.val();
+        let portfolio_id = null;
+        let portfolio_selected = false;
+        // portfolio will be either readonly or a dropdown, handle both cases
+        if (portfolioDropdown.length) { // need to test length since the query will always be defined, even if not in DOM
+            // Retrieve the selected portfolio ID
+            portfolio_id = portfolioDropdown.val();
+            if (portfolio_id) {
+                portfolio_selected = true;
+            }
+        } else {
+            // get readonly field value
+            let portfolio = portfolioField.querySelector(".readonly").innerText;
+            if (portfolio != "-") {
+                portfolio_selected = true;
+            }
+        }
 
-        if (portfolio_id) {
+        if (portfolio_selected) {
             // A portfolio is selected - update suborganization dropdown and show/hide relevant fields
 
-            // Update suborganization dropdown for the selected portfolio
-            updateSubOrganizationDropdown(portfolio_id);
+            if (portfolio_id) {
+                // Update suborganization dropdown for the selected portfolio
+                updateSubOrganizationDropdown(portfolio_id);
+            }
 
             // Show fields relevant to a selected portfolio
             if (suborganizationField) showElement(suborganizationField);
@@ -468,10 +486,22 @@ export function handlePortfolioSelection(
      * This function ensures the form dynamically reflects whether a specific suborganization is being selected or requested.
      */
     function updateSuborganizationFieldsDisplay() {
-        let portfolio_id = portfolioDropdown.val();
+        let portfolio_selected = false;
+        // portfolio will be either readonly or a dropdown, handle both cases
+        if (portfolioDropdown.length) { // need to test length since the query will always be defined, even if not in DOM
+            // Retrieve the selected portfolio ID
+            if (portfolioDropdown.val()) {
+                portfolio_selected = true;
+            }
+        } else {
+            // get readonly field value
+            if (portfolioField.querySelector(".readonly").innerText != "-") {
+                portfolio_selected = true;
+            }
+        }
         let suborganization_id = suborganizationDropdown.val();
 
-        if (portfolio_id && !suborganization_id) {
+        if (portfolio_selected && !suborganization_id) {
             // Show suborganization request fields
             if (requestedSuborganizationField) showElement(requestedSuborganizationField);
             if (suborganizationCity) showElement(suborganizationCity);

From 4ac2250c084e6b851ceb073776bf9381c64d41ef Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 11:03:51 -0400
Subject: [PATCH 46/47] fixed bug in status check in javascript on domain
 request form

---
 src/registrar/assets/src/js/getgov-admin/domain-request-form.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
index 16acaf91c..b9084494a 100644
--- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js
@@ -383,7 +383,7 @@ class CustomizableEmailBase {
     initializeFormGroups() {
         let isStatus = false;
         if (this.statusSelect) {
-            this.statusSelect.value == this.statusToCheck;
+            isStatus = this.statusSelect.value == this.statusToCheck;
         } else {
             // statusSelect does not exist, indicating readonly
             if (this.dropdownFormGroup) {

From 4502497037aa36632b0eab735bef93904120a69b Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Tue, 18 Mar 2025 11:52:54 -0400
Subject: [PATCH 47/47] fixed javascript on readonly fields on portfolio form

---
 .../assets/src/js/getgov-admin/portfolio-form.js   | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
index 2e437c520..7777dabdb 100644
--- a/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
+++ b/src/registrar/assets/src/js/getgov-admin/portfolio-form.js
@@ -21,6 +21,8 @@ function handlePortfolioFields(){
     const federalTypeField = document.querySelector(".field-federal_type");
     const urbanizationField = document.querySelector(".field-urbanization");
     const stateTerritoryDropdown = document.getElementById("id_state_territory");
+    const stateTerritoryField = document.querySelector(".field-state_territory");
+    const stateTerritoryReadonly = stateTerritoryField.querySelector(".readonly");
     const seniorOfficialAddUrl = document.getElementById("senior-official-add-url").value;
     const seniorOfficialApi = document.getElementById("senior_official_from_agency_json_url").value;
     const federalPortfolioApi = document.getElementById("federal_and_portfolio_types_from_agency_json_url").value;
@@ -85,9 +87,9 @@ function handlePortfolioFields(){
      * 2. else show org name, hide federal agency, hide federal type if applicable
      */
     function handleOrganizationTypeChange() {
-        if (organizationTypeDropdown && organizationNameField) {
-            let selectedValue = organizationTypeDropdown.value;
-            if (selectedValue === "federal") {
+        if (organizationTypeField && organizationNameField) {
+            let selectedValue = organizationTypeDropdown ? organizationTypeDropdown.value : organizationTypeReadonly.innerText;
+            if (selectedValue === "federal" || selectedValue === "Federal") {
                 hideElement(organizationNameField);
                 showElement(federalAgencyField);
                 if (federalTypeField) {
@@ -207,8 +209,8 @@ function handlePortfolioFields(){
      * Handle urbanization
      */
     function handleStateTerritoryChange() {
-        let selectedValue = stateTerritoryDropdown.value;
-        if (selectedValue === "PR") {
+        let selectedValue = stateTerritoryDropdown ? stateTerritoryDropdown.value : stateTerritoryReadonly.innerText;
+        if (selectedValue === "PR" || selectedValue === "Puerto Rico (PR)") {
             showElement(urbanizationField)
         } else {
             hideElement(urbanizationField)
@@ -265,7 +267,7 @@ function handlePortfolioFields(){
      * Initializes necessary data and display configurations for the portfolio fields.
      */
     function initializePortfolioSettings() {
-        if (urbanizationField && stateTerritoryDropdown) {
+        if (urbanizationField && stateTerritoryField) {
             handleStateTerritoryChange();
         }
         handleOrganizationTypeChange();