Merge pull request #5666 from raft-tech/5621-add-regional-user-support-for-stt-feedback-reports-page

implement regional feedback reports support

Author: mattcoleanderson

tdrs-backend/tdpservice/reports/test/conftest.py

@@ -174,3 +174,27 @@ def bad_report_source_data(bad_report_source_zip_file, fake_file_name):
         "file": bad_report_source_zip_file,
         "original_filename": fake_file_name,
     }
+
+
+@pytest.fixture
+def regional_report_file_instance(regional_user, stt):
+    """Return a persisted ReportFile tied to an STT in the regional user's region."""
+    return ReportFileFactory.create(
+        stt=stt,
+        user=regional_user,
+    )
+
+
+@pytest.fixture
+def other_region_report_file_instance(regional_user):
+    """Return a persisted ReportFile tied to an STT outside the regional user's region."""
+    from tdpservice.stts.models import STT, Region
+
+    other_region, _ = Region.objects.get_or_create(id=99)
+    other_stt, _ = STT.objects.get_or_create(
+        name="Other Region STT", region=other_region, stt_code="99"
+    )
+    return ReportFileFactory.create(
+        stt=other_stt,
+        user=regional_user,
+    )

tdrs-backend/tdpservice/reports/test/test_views.py

@@ -347,3 +347,81 @@ def test_latest_param_returns_empty_when_no_reports(
 
         assert resp.status_code == status.HTTP_200_OK
         assert len(resp.data["results"]) == 0
+
+
+@pytest.mark.django_db
+class TestReportFileViewAsRegionalStaff:
+    """Tests for an OFA Regional Staff user interacting with /v1/reports/ endpoints."""
+
+    root_url = "/v1/reports/"
+
+    @pytest.fixture
+    def api_client_logged_in(self, api_client, regional_user):
+        """Return an API client authenticated as a regional staff user."""
+        api_client.login(username=regional_user.username, password="test_password")
+        return api_client
+
+    def test_can_list_reports(
+        self, api_client_logged_in, regional_report_file_instance
+    ):
+        """Regional staff can list report files (200 OK)."""
+        resp = api_client_logged_in.get(self.root_url)
+        assert resp.status_code == status.HTTP_200_OK
+
+    def test_only_sees_reports_in_own_region(
+        self,
+        api_client_logged_in,
+        regional_user,
+        regional_report_file_instance,
+        other_region_report_file_instance,
+    ):
+        """Regional staff only sees reports for STTs in their region."""
+        resp = api_client_logged_in.get(self.root_url)
+
+        assert resp.status_code == status.HTTP_200_OK
+
+        returned_ids = [row["id"] for row in resp.data["results"]]
+        assert regional_report_file_instance.id in returned_ids
+        assert other_region_report_file_instance.id not in returned_ids
+
+    def test_cannot_create_report_files(
+        self, api_client_logged_in, report_file_data
+    ):
+        """Regional staff cannot create report files (403)."""
+        resp = api_client_logged_in.post(
+            self.root_url, report_file_data, format="multipart"
+        )
+        assert resp.status_code == status.HTTP_403_FORBIDDEN
+
+    def test_can_download_report_in_own_region(
+        self, api_client_logged_in, regional_report_file_instance
+    ):
+        """Regional staff can download report files for STTs in their region."""
+        resp = api_client_logged_in.get(
+            f"{self.root_url}{regional_report_file_instance.id}/download/"
+        )
+        assert resp.status_code == status.HTTP_200_OK
+
+    def test_cannot_download_report_outside_region(
+        self, api_client_logged_in, other_region_report_file_instance
+    ):
+        """Regional staff cannot download report files outside their region."""
+        resp = api_client_logged_in.get(
+            f"{self.root_url}{other_region_report_file_instance.id}/download/"
+        )
+        # Returns 404 because get_queryset filters out reports outside the region
+        assert resp.status_code == status.HTTP_404_NOT_FOUND
+
+    def test_stt_query_param_filters_results(
+        self,
+        api_client_logged_in,
+        regional_user,
+        regional_report_file_instance,
+        stt,
+    ):
+        """Regional staff can filter reports by STT query param."""
+        resp = api_client_logged_in.get(f"{self.root_url}?stt={stt.id}")
+
+        assert resp.status_code == status.HTTP_200_OK
+        returned_ids = [row["id"] for row in resp.data["results"]]
+        assert regional_report_file_instance.id in returned_ids

tdrs-backend/tdpservice/reports/views.py

@@ -27,10 +27,18 @@ def get_queryset(self):
         if self.request.user.is_data_analyst and hasattr(self.request.user, 'stt'):
             queryset = queryset.filter(stt=self.request.user.stt)
 
+        # Regional Staff should only see reports for STTs in their region
+        if self.request.user.is_regional_staff and hasattr(self.request.user, 'regions'):
+            user_regions = self.request.user.regions.all()
+            queryset = queryset.filter(stt__region__in=user_regions)
+
         # Query params for adding additional filters to queryset
         year = self.request.query_params.get('year')
         latest = self.request.query_params.get('latest')
+        stt = self.request.query_params.get('stt')
 
+        if stt:
+            queryset = queryset.filter(stt_id=stt)
         if year:
             queryset = queryset.filter(year=year)
         if latest and latest.lower() == 'true':

tdrs-backend/tdpservice/users/migrations/0056_add_regional_staff_report_permissions.py

@@ -0,0 +1,52 @@
+from django.db import migrations
+
+from tdpservice.users.permissions import (
+    create_perms,
+    get_permission_ids_for_model,
+    view_permissions_q,
+)
+
+
+def set_regional_staff_permissions(apps, schema_editor):
+    """Add view_reportfile permission to OFA Regional Staff group."""
+    regional_staff = apps.get_model("auth", "Group").objects.get(
+        name="OFA Regional Staff"
+    )
+
+    report_file_permissions = get_permission_ids_for_model(
+        "reports", "reportfile", filters=[view_permissions_q]
+    )
+
+    regional_staff.permissions.add(*report_file_permissions)
+
+
+def unset_regional_staff_permissions(apps, schema_editor):
+    """Remove view_reportfile permission from OFA Regional Staff group."""
+    regional_staff = apps.get_model("auth", "Group").objects.get(
+        name="OFA Regional Staff"
+    )
+
+    report_file_permissions = get_permission_ids_for_model(
+        "reports", "reportfile", filters=[view_permissions_q]
+    )
+
+    regional_staff.permissions.remove(*report_file_permissions)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("auth", "__latest__"),
+        ("users", "0055_add_ofa_sys_admin_report_permissions"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            create_perms,
+            reverse_code=migrations.RunPython.noop,
+        ),
+        migrations.RunPython(
+            set_regional_staff_permissions,
+            reverse_code=unset_regional_staff_permissions,
+        ),
+    ]

tdrs-backend/tdpservice/users/migrations/0057_merge_20260318_1613.py

@@ -0,0 +1,14 @@
+# Generated by Django 5.2.10 on 2026-03-18 16:13
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0056_add_regional_staff_report_permissions'),
+        ('users', '0056_historicaluser_historicalregionmeta_and_more'),
+    ]
+
+    operations = [
+    ]

tdrs-backend/tdpservice/users/migrations/0058_merge_20260318_2105.py

@@ -0,0 +1,14 @@
+# Generated by Django 5.2.10 on 2026-03-18 21:05
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0057_alter_userchangerequest_status'),
+        ('users', '0057_merge_20260318_1613'),
+    ]
+
+    operations = [
+    ]

tdrs-backend/tdpservice/users/permissions.py

@@ -180,20 +180,31 @@ def has_permission(self, request, view):
                 return False
 
         # Data Analysts are limited to only report files for their designated STT
-        # NOTE: Will Regional Staff use report files?
         if has_permission and request.user.is_data_analyst:
             return is_own_stt(request.user, get_requested_stt(request, view))
 
+        # Regional Staff are not allowed to create report files
+        # but can list and download reports for STTs in their region
+        if has_permission and request.user.is_regional_staff:
+            if hasattr(view, "action") and view.action in ["create"]:
+                return False
+            return is_own_region(request.user, get_requested_stt(request, view))
+
         return has_permission
 
     def has_object_permission(self, request, view, obj):
-        """Check if a user cn interact with a specific report file, based on STT."""
+        """Check if a user can interact with a specific report file, based on STT."""
         # Data Analysts can only see report files uploaded for their designated STT
         if request.user.is_data_analyst:
             user_stt = request.user.stt.id if hasattr(request.user, "stt") else None
             return user_stt == obj.stt_id
 
-        # NOTE: Will Regional Staff user report files?
+        # Regional Staff can only see report files for STTs in their region
+        if request.user.is_regional_staff:
+            user_regions = (
+                request.user.regions.all() if hasattr(request.user, "regions") else None
+            )
+            return obj.stt.region in user_regions
 
         return super().has_object_permission(request, view, obj)
 

tdrs-frontend/cypress/e2e/feedback-reports/admin-feedback-reports.feature

@@ -59,8 +59,3 @@ Feature: Admin Feedback Reports
         And 'Admin Alex' selects fiscal year '2025'
         Then 'Admin Alex' sees the upload form for fiscal year '2025'
 
-    # Permission tests - users who should NOT have access
-
-    Scenario: OFA Regional Staff cannot see Feedback Reports nav item
-        Given 'Regional Staff Cypress' logs in
-        Then 'Regional Staff Cypress' does not see Feedback Reports in the navigation

tdrs-frontend/cypress/e2e/feedback-reports/admin-feedback-reports.js

@@ -133,13 +133,3 @@ Then('{string} sees the error about missing date', () => {
   cy.contains(fr.ERROR_MESSAGES.NO_DATE).should('exist')
 })
 
-// ──────────────────────────────────────────────────────────
-// Permission Tests - No Access
-// ──────────────────────────────────────────────────────────
-
-Then('{string} does not see Feedback Reports in the navigation', () => {
-  cy.visit('/')
-  cy.contains('Welcome to TDP', { timeout: 30000 }).should('exist')
-  cy.get('.usa-nav__primary').should('exist')
-  cy.get('.usa-nav__primary').contains('Feedback Reports').should('not.exist')
-})

tdrs-frontend/src/components/FeedbackReports/FeedbackReportAlert.jsx

@@ -27,7 +27,7 @@ const saveDismissedState = (year, reportCreatedAt) => {
  * Fetches the latest report internally using the `latest=true` query param.
  * Can be dismissed by the user, with state persisted in localStorage per fiscal year.
  */
-const FeedbackReportAlert = () => {
+const FeedbackReportAlert = ({ stt = null }) => {
   const { yearInputValue, quarterInputValue } = useReportsContext()
   const [latestReportDate, setLatestReportDate] = useState(null)
   const [isDismissed, setIsDismissed] = useState(false)
@@ -40,15 +40,18 @@ const FeedbackReportAlert = () => {
         return
       }
 
+      const params = {
+        year: yearInputValue,
+        quarter: quarterInputValue,
+        latest: 'true',
+      }
+      if (stt) {
+        params.stt = stt.id
+      }
+
       const { data, ok, error } = await get(
         `${process.env.REACT_APP_BACKEND_URL}/reports/`,
-        {
-          params: {
-            year: yearInputValue,
-            quarter: quarterInputValue,
-            latest: 'true',
-          },
-        }
+        { params }
       )
 
       if (ok && data?.results?.length > 0) {
@@ -74,7 +77,7 @@ const FeedbackReportAlert = () => {
     }
 
     fetchLatestFeedbackReport()
-  }, [yearInputValue, quarterInputValue])
+  }, [yearInputValue, quarterInputValue, stt])
 
   const handleDismiss = useCallback(() => {
     if (yearInputValue && latestReportDate) {
@@ -93,7 +96,12 @@ const FeedbackReportAlert = () => {
   })
 
   return (
-    <div className="usa-alert usa-alert--info margin-top-4 margin-bottom-4">
+    <div
+      className="usa-alert usa-alert--info margin-top-4 margin-bottom-4"
+      role="region"
+      aria-labelledby="feedback-alert-text"
+      tabIndex={-1}
+    >
       <div
         className="usa-alert__body"
         style={{
@@ -102,9 +110,13 @@ const FeedbackReportAlert = () => {
           alignItems: 'flex-start',
         }}
       >
-        <p className="usa-alert__text">
+        <p className="usa-alert__text" id="feedback-alert-text">
           Feedback Reports Available as of {formattedDate}. Please{' '}
-          <a href={`/feedback-reports?year=${yearInputValue}`}>
+          <a
+            href={`/feedback-reports?year=${yearInputValue}${stt ? `&stt=${stt.name}` : ''}`}
+            target="_blank"
+            rel="noopener noreferrer"
+          >
             review the feedback
           </a>{' '}
           and if needed, resubmit complete and accurate data.