diff --git a/course_discovery/apps/api/v2/views/catalog_queries.py b/course_discovery/apps/api/v2/views/catalog_queries.py index 62c96a1662..589f58108c 100644 --- a/course_discovery/apps/api/v2/views/catalog_queries.py +++ b/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 diff --git a/course_discovery/apps/course_metadata/models.py b/course_discovery/apps/course_metadata/models.py index f0979f1122..3665682167 100644 --- a/course_discovery/apps/course_metadata/models.py +++ b/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. diff --git a/course_discovery/apps/course_metadata/tests/management/test_mixins.py b/course_discovery/apps/course_metadata/tests/management/test_mixins.py new file mode 100644 index 0000000000..d7908243d4 --- /dev/null +++ b/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 + ) diff --git a/course_discovery/apps/course_metadata/tests/test_models.py b/course_discovery/apps/course_metadata/tests/test_models.py index 3145126649..1ea16107fd 100644 --- a/course_discovery/apps/course_metadata/tests/test_models.py +++ b/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: ") - - -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)