Improving cog/over search results (#4528)

* Improving cog/over search results

* Restoring deleting log line

* Accepting empty string or either

backend/dissemination/search.py

@@ -4,6 +4,7 @@
 from .searchlib.search_constants import ORDER_BY, DIRECTION, DAS_LIMIT
 from .searchlib.search_general import report_timing, search_general
 from .searchlib.search_alns import search_alns
+from .searchlib.search_cog_or_oversight import search_cog_or_oversight
 from .searchlib.search_findings import search_findings
 from .searchlib.search_direct_funding import search_direct_funding
 from .searchlib.search_major_program import search_major_program
@@ -64,6 +65,7 @@ def search(params):
         logger.info("search Searching `DisseminationCombined`")
         results = search_general(DisseminationCombined, params)
         results = search_alns(results, params)
+        results = search_cog_or_oversight(results, params)
         results = search_findings(results, params)
         results = search_direct_funding(results, params)
         results = search_major_program(results, params)

backend/dissemination/searchlib/search_cog_or_oversight.py

@@ -0,0 +1,47 @@
+from .search_general import report_timing
+
+from django.db.models import Q
+
+import logging
+import time
+
+logger = logging.getLogger(__name__)
+
+
+def search_cog_or_oversight(general_results, params):
+    t0 = time.time()
+
+    q_cogover = _get_cog_or_oversight_match_query(
+        params.get("agency_name", None), params.get("cog_or_oversight", "either")
+    )
+    filtered_general_results = general_results.filter(q_cogover)
+
+    t1 = time.time()
+    report_timing("search_cog_or_oversight", params, t0, t1)
+
+    return filtered_general_results
+
+
+def _get_cog_or_oversight_match_query(agency_name, cog_or_oversight):
+    if cog_or_oversight.lower() in ["", "either"]:
+        if agency_name:
+            return Q(
+                Q(cognizant_agency__in=[agency_name])
+                | Q(oversight_agency__in=[agency_name])
+            )
+        else:
+            # Every submission should have a value for either cog or over, so
+            # nothing to do here
+            return Q()
+    elif cog_or_oversight.lower() == "cog":
+        if agency_name:
+            return Q(cognizant_agency__in=[agency_name])
+        else:
+            # Submissions that have any cog
+            return Q(cognizant_agency__isnull=False)
+    elif cog_or_oversight.lower() == "oversight":
+        if agency_name:
+            return Q(oversight_agency__in=[agency_name])
+        else:
+            # Submissions that have any over
+            return Q(oversight_agency__isnull=False)

backend/dissemination/searchlib/search_general.py

@@ -44,13 +44,6 @@ def search_general(base_model, params=None):
     q_end_date = _get_end_date_match_query(params.get("end_date", None))
     r_end_date = base_model.objects.filter(q_end_date)
 
-    ##############
-    # Cog/Over
-    q_cogover = _get_cog_or_oversight_match_query(
-        params.get("agency_name", None), params.get("cog_or_oversight", None)
-    )
-    r_cogover = base_model.objects.filter(q_cogover)
-
     ##############
     # Fiscal year end month
     q_fy_end_month = _get_fy_end_month_match_query(params.get("fy_end_month", None))
@@ -74,7 +67,6 @@ def search_general(base_model, params=None):
         & r_start_date
         & r_end_date
         & r_state
-        & r_cogover
         & r_names
         & r_uei
         & r_base
@@ -130,21 +122,6 @@ def _get_end_date_match_query(end_date):
     return Q(fac_accepted_date__lte=end_date)
 
 
-def _get_cog_or_oversight_match_query(agency_name, cog_or_oversight):
-    if not agency_name:
-        return Q()
-
-    if cog_or_oversight.lower() == "either":
-        return Q(
-            Q(cognizant_agency__in=[agency_name])
-            | Q(oversight_agency__in=[agency_name])
-        )
-    elif cog_or_oversight.lower() == "cog":
-        return Q(cognizant_agency__in=[agency_name])
-    elif cog_or_oversight.lower() == "oversight":
-        return Q(oversight_agency__in=[agency_name])
-
-
 def _get_audit_years_match_query(audit_years):
     if not audit_years:
         return Q()

backend/dissemination/test_search.py

@@ -283,50 +283,6 @@ def test_date_range(self):
             self.assertGreaterEqual(r.fac_accepted_date, search_start_date)
             self.assertLessEqual(r.fac_accepted_date, search_end_date)
 
-    def test_cognizant_agency(self):
-        """
-        Given a cognizant agency name, search_general should return only records with a matching cognizant agency name (not oversight)
-        """
-
-        baker.make(General, is_public=True, cognizant_agency="01")
-        baker.make(General, is_public=True, cognizant_agency="02")
-
-        baker.make(General, is_public=True, oversight_agency="01")
-
-        results = search_general(
-            General,
-            {
-                "cog_or_oversight": "cog",
-                "agency_name": "01",
-            },
-        )
-
-        assert_all_results_public(self, results)
-        self.assertEqual(len(results), 1)
-        self.assertEqual(results[0].cognizant_agency, "01")
-
-    def test_oversight_agency(self):
-        """
-        Given an oversight agency name, search_general should return only records with a matching oversight agency name (not cognizant)
-        """
-
-        baker.make(General, is_public=True, cognizant_agency="01")
-
-        baker.make(General, is_public=True, oversight_agency="01")
-        baker.make(General, is_public=True, oversight_agency="02")
-
-        results = search_general(
-            General,
-            {
-                "cog_or_oversight": "oversight",
-                "agency_name": "01",
-            },
-        )
-
-        assert_all_results_public(self, results)
-        self.assertEqual(len(results), 1)
-        self.assertEqual(results[0].oversight_agency, "01")
-
     def test_audit_year(self):
         """
         Given a list of audit years, search_general should return only records where
@@ -671,6 +627,54 @@ def test_alns_no_findings(self):
 
 
 class SearchAdvancedFilterTests(TestMaterializedViewBuilder):
+    def test_search_cog_or_oversight(self):
+        """
+        When making a search on major program, search_general should only return records with an award of that type.
+        """
+        general_cog = baker.make(
+            General,
+            cognizant_agency=42,
+            oversight_agency=None,
+        )
+        baker.make(FederalAward, report_id=general_cog)
+        general_over = baker.make(
+            General,
+            cognizant_agency=None,
+            oversight_agency=24,
+        )
+        baker.make(FederalAward, report_id=general_over)
+        self.refresh_materialized_view()
+
+        adv_params = {"advanced_search_flag": True}
+
+        # Either cog or over with no agency should return all
+        params = {"agency_name": None, "cog_or_oversight": "either", **adv_params}
+        results = search(params)
+        self.assertEqual(len(results), 2)
+
+        # Unused agency should return nothing
+        params = {"agency_name": 99, "cog_or_oversight": "either", **adv_params}
+        results = search(params)
+        self.assertEqual(len(results), 0)
+
+        # Either cog or over with valid agency
+        params = {"agency_name": 42, "cog_or_oversight": "either", **adv_params}
+        results = search(params)
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].report_id, general_cog.report_id)
+
+        # Cog with valid agency
+        params = {"agency_name": 42, "cog_or_oversight": "cog", **adv_params}
+        results = search(params)
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].report_id, general_cog.report_id)
+
+        # Over with valid agency
+        params = {"agency_name": 24, "cog_or_oversight": "oversight", **adv_params}
+        results = search(params)
+        self.assertEqual(len(results), 1)
+        self.assertEqual(results[0].report_id, general_over.report_id)
+
     def test_search_findings(self):
         """
         When making a search on a particular type of finding, search_general should only return records with a finding of that type.