From 7dd1b2c572be2970d235bc3f6aab4552ded0f5dc Mon Sep 17 00:00:00 2001
From: Caleb Godwin <cgodwin@fearless.tech>
Date: Mon, 2 Dec 2024 14:41:15 -0500
Subject: [PATCH] fix: only count approved resources in subjects endpoint

---
 .../resources/tests/test_subjects_endpoint.py | 74 +++++++++++++++++++
 solution/backend/resources/views/subjects.py  | 12 +--
 2 files changed, 81 insertions(+), 5 deletions(-)
 create mode 100644 solution/backend/resources/tests/test_subjects_endpoint.py

diff --git a/solution/backend/resources/tests/test_subjects_endpoint.py b/solution/backend/resources/tests/test_subjects_endpoint.py
new file mode 100644
index 0000000000..5b9fb936c1
--- /dev/null
+++ b/solution/backend/resources/tests/test_subjects_endpoint.py
@@ -0,0 +1,74 @@
+
+from django.contrib.auth.models import User
+from django.test import TestCase
+from rest_framework.test import APIClient
+
+from resources.models import (
+    FederalRegisterLink,
+    InternalFile,
+    Subject,
+)
+
+
+class TestResourcesEndpoint(TestCase):
+    def login(self):
+        self.client = APIClient()
+        self.user = User.objects.create_superuser(username='test_user', password='test')  # noqa: S106
+        self.client.force_authenticate(self.user)
+
+    def setUp(self):
+        subject1 = Subject.objects.create(full_name="Access to Services", short_name="Subj1", abbreviation="ATS")
+        subject2 = Subject.objects.create(full_name="Subject 2", short_name="Subj2", abbreviation="S2")
+
+        f1 = FederalRegisterLink.objects.create(approved=True)
+        f1.subjects.set([subject1])
+        f1.save()
+
+        f2 = FederalRegisterLink.objects.create(approved=True)
+        f2.subjects.set([subject1])
+        f2.save()
+
+        f3 = FederalRegisterLink.objects.create(approved=False)
+        f3.subjects.set([subject1, subject2])
+        f3.save()
+
+        file1 = InternalFile.objects.create(approved=True)
+        file1.subjects.set([subject1])
+        file1.save()
+
+        file2 = InternalFile.objects.create(approved=False)
+        file2.subjects.set([subject1, subject2])
+        file2.save()
+
+    def test_subjects_endpoint_logged_in(self):
+        self.login()
+        data = self.client.get("/v3/resources/subjects").data
+        self.assertEqual(data["count"], 2)
+
+        self.assertEqual(data["results"][0]["public_resources"], 2)
+        self.assertEqual(data["results"][0]["internal_resources"], 1)
+        self.assertEqual(data["results"][0]["full_name"], "Access to Services")
+        self.assertEqual(data["results"][0]["short_name"], "Subj1")
+        self.assertEqual(data["results"][0]["abbreviation"], "ATS")
+
+        self.assertEqual(data["results"][1]["public_resources"], 0)
+        self.assertEqual(data["results"][1]["internal_resources"], 0)
+        self.assertEqual(data["results"][1]["full_name"], "Subject 2")
+        self.assertEqual(data["results"][1]["short_name"], "Subj2")
+        self.assertEqual(data["results"][1]["abbreviation"], "S2")
+
+    def test_subjects_endpoint_logged_out(self):
+        data = self.client.get("/v3/resources/subjects").data
+        self.assertEqual(data["count"], 2)
+
+        self.assertEqual(data["results"][0]["public_resources"], 2)
+        self.assertEqual(data["results"][0]["internal_resources"], 0)
+        self.assertEqual(data["results"][0]["full_name"], "Access to Services")
+        self.assertEqual(data["results"][0]["short_name"], "Subj1")
+        self.assertEqual(data["results"][0]["abbreviation"], "ATS")
+
+        self.assertEqual(data["results"][1]["public_resources"], 0)
+        self.assertEqual(data["results"][1]["internal_resources"], 0)
+        self.assertEqual(data["results"][1]["full_name"], "Subject 2")
+        self.assertEqual(data["results"][1]["short_name"], "Subj2")
+        self.assertEqual(data["results"][1]["abbreviation"], "S2")
diff --git a/solution/backend/resources/views/subjects.py b/solution/backend/resources/views/subjects.py
index 170e7b3905..2c276a949f 100644
--- a/solution/backend/resources/views/subjects.py
+++ b/solution/backend/resources/views/subjects.py
@@ -9,9 +9,9 @@
 
 @extend_schema(
     tags=["resources/metadata"],
-    description="Retrieve a list of subjects, each annotated with the count of associated public and internal resources. "
-                "Authenticated users can see the count of both public and internal resources, while unauthenticated users "
-                "only see the count of public resources. The subjects are ordered by a predefined sort order.",
+    description="Retrieve a list of subjects, each annotated with the count of associated approved public and internal "
+                "resources. Authenticated users can see the count of both public and internal resources, while unauthenticated "
+                "users only see the count of public resources. The subjects are ordered by a predefined sort order.",
     responses={200: SubjectWithCountsSerializer(many=True)},
 )
 class SubjectViewSet(viewsets.ReadOnlyModelViewSet):
@@ -19,10 +19,12 @@ class SubjectViewSet(viewsets.ReadOnlyModelViewSet):
     pagination_class = ViewSetPagination
 
     def get_queryset(self):
+        public_filter = Q(resources__abstractpublicresource__isnull=False) & Q(resources__approved=True)
+        internal_filter = Q(resources__abstractinternalresource__isnull=False) & Q(resources__approved=True)
         return Subject.objects.annotate(**{
-            "public_resources": Count("resources", filter=Q(resources__abstractpublicresource__isnull=False)),
+            "public_resources": Count("resources", filter=public_filter),
             "internal_resources": (
-                Count("resources", filter=Q(resources__abstractinternalresource__isnull=False))
+                Count("resources", filter=internal_filter)
                 if self.request.user.is_authenticated else
                 Value(0)
             ),