address comments and quality

course_discovery/apps/api/v2/views/catalog_queries.py

@@ -43,15 +43,15 @@ def get(self, request):
                 course_run_ids = course_run_ids.split(',')
                 specified_course_ids = course_run_ids
                 identified_course_ids.update(
-                    i.key
-                    for i in self.search(
+                    self.search(
                         query,
                         queryset=CourseRun.objects.all(),
                         partner=ESDSLQ('term', partner=partner.short_code),
                         identifiers=ESDSLQ('terms', **{'key.raw': course_run_ids}),
                         document=CourseRunDocument
-                    )
+                    ).values_list('key', flat=True)
                 )
+
             if course_uuids:
                 course_uuids = [UUID(course_uuid) for course_uuid in course_uuids.split(',')]
                 specified_course_ids += course_uuids

course_discovery/apps/course_metadata/models.py

@@ -1173,6 +1173,8 @@ def search(cls, query, queryset=None, page_size=settings.ELASTICSEARCH_DSL_QUERY
         query = clean_query(query)
         queryset = queryset or cls.objects.all()
 
+        import pdb
+        pdb.set_trace()
         if query == '(*)':
             # Early-exit optimization. Wildcard searching is very expensive in elasticsearch. And since we just
             # want everything, we don't need to actually query elasticsearch at all.

course_discovery/apps/course_metadata/tests/management/test_mixins.py

@@ -0,0 +1,44 @@
+from unittest.mock import patch
+
+from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
+from django.test import TestCase
+from course_discovery.apps.course_metadata.tests import factories
+from course_discovery.apps.course_metadata.search_indexes.documents import CourseDocument
+
+
+class TestSearchAfterMixin(ElasticsearchTestMixin, TestCase):
+    """
+    Unit tests for SearchAfterMixin.
+    Uses a proxy model `CourseProxy` that extends this mixin so we can replicate the behavior for Courses.
+    """
+    def setUp(self):
+        super().setUp()
+
+        self.total_courses = 5
+        factories.CourseFactory.create_batch(self.total_courses)
+
+    @patch("course_discovery.apps.course_metadata.models.registry.get_documents")
+    def test_fetch_all_courses(self, mock_get_documents):
+        query = 'Course*'
+        mock_get_documents.return_value = [CourseDocument]
+
+        queryset = factories.CourseProxy.search(query=query, page_size=2)
+
+        unique_items = set(queryset)
+        self.assertEqual(len(queryset), len(unique_items), 'Queryset contains duplicate entries.')
+        self.assertEqual(len(queryset), self.total_courses)
+
+    def test_wildcard_query_early_exit(self):
+        """
+        Test the early exit optimization when the query is `(*)`.
+        """
+        query = '*'
+
+        queryset = factories.CourseProxy.search(query=query)
+
+        self.assertEqual(len(queryset), self.total_courses)
+        self.assertQuerysetEqual(
+            queryset.order_by("id"),
+            factories.Course.objects.all().order_by("id"),
+            transform=lambda x: x
+        )

course_discovery/apps/course_metadata/tests/test_models.py

@@ -30,7 +30,6 @@
 from course_discovery.apps.api.v1.tests.test_views.mixins import OAuth2Mixin
 from course_discovery.apps.core.models import Currency
 from course_discovery.apps.core.tests.helpers import make_image_file
-from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
 from course_discovery.apps.core.utils import SearchQuerySetWrapper
 from course_discovery.apps.course_metadata.choices import (
     CourseRunRestrictionType, CourseRunStatus, ExternalProductStatus, ProgramStatus
@@ -45,13 +44,12 @@
 from course_discovery.apps.course_metadata.publishers import (
     CourseRunMarketingSitePublisher, ProgramMarketingSitePublisher
 )
-from course_discovery.apps.course_metadata.search_indexes.documents import CourseDocument
 from course_discovery.apps.course_metadata.signals import (
     connect_course_data_modified_timestamp_related_models, disconnect_course_data_modified_timestamp_related_models
 )
 from course_discovery.apps.course_metadata.tests import factories
 from course_discovery.apps.course_metadata.tests.factories import (
-    AdditionalMetadataFactory, CourseFactory, CourseProxy, CourseRunFactory, CourseTypeFactory, CourseUrlSlugFactory,
+    AdditionalMetadataFactory, CourseFactory, CourseRunFactory, CourseTypeFactory, CourseUrlSlugFactory,
     ImageFactory, OrganizationFactory, PartnerFactory, ProgramFactory, SeatFactory, SeatTypeFactory, SourceFactory,
     SubjectFactory
 )
@@ -4195,23 +4193,3 @@ def test_basic(self):
         self.assertEqual(course_run.restricted_run, restricted_course_run)
         self.assertEqual(restricted_course_run.restriction_type, 'custom-b2b-enterprise')
         self.assertEqual(str(restricted_course_run), "course-v1:SC+BreadX+3T2015: <custom-b2b-enterprise>")
-
-
-class TestSearchAfterMixin(ElasticsearchTestMixin, TestCase):
-    def setUp(self):
-        super().setUp()
-
-        self.total_courses = 5
-        for _ in range(self.total_courses):
-            CourseFactory()
-
-    @patch("course_discovery.apps.course_metadata.models.registry.get_documents")
-    def test_fetch_all_courses(self, mock_get_documents):
-        query = "Course*"
-        mock_get_documents.return_value = [CourseDocument]
-
-        queryset = CourseProxy.search(query=query, page_size=2)
-
-        unique_items = set(queryset)
-        self.assertEqual(len(queryset), len(unique_items), "Queryset contains duplicate entries.")
-        self.assertEqual(len(queryset), self.total_courses)