Skip to content

Commit

Permalink
fix: sorting of elements in api/v2/search/all (#4539)
Browse files Browse the repository at this point in the history
  • Loading branch information
AfaqShuaib09 authored Jan 14, 2025
1 parent 727e828 commit 0ab4e42
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 8 deletions.
42 changes: 42 additions & 0 deletions course_discovery/apps/api/v2/tests/test_views/test_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,3 +136,45 @@ def test_search_after_pagination(self):
assert (
single_page_data["results"] == all_results
), "Combined pagination results do not match single request results"

def test_last_item_sort_pagination(self):
"""
Test that the results won't be duplicated in any request response.
"""
page_size = 5
PersonFactory.create_batch(10, partner=self.partner)
courses = CourseFactory.create_batch(10, partner=self.partner)

for course in courses:
CourseRunFactory(
course__partner=self.partner,
course=course,
type__is_marketable=True,
status=CourseRunStatus.Published,
)
total_items_count = 30 # 10 Persons + 10 Courses + 10 CourseRuns
max_pages = total_items_count // page_size

response_data = self.fetch_page_data(page_size)
all_results = response_data["results"]
aggregate_uuids = [result["aggregation_uuid"] for result in all_results]
next_url = response_data.get("next")

for _ in range(max_pages - 1):
parsed_url = urlparse(next_url)
query_params = parse_qs(parsed_url.query)
search_after = query_params.get("search_after", [None])[0]
assert search_after is not None

last_sort_value = all_results[-1]["sort"]
assert json.loads(search_after) == last_sort_value

response_data = self.fetch_page_data(page_size, next_url=next_url)
for result in response_data['results']:
assert result['aggregation_uuid'] not in aggregate_uuids
aggregate_uuids.append(result['aggregation_uuid'])

all_results.extend(response_data["results"])
next_url = response_data.get("next")

assert len(aggregate_uuids) == total_items_count
Original file line number Diff line number Diff line change
Expand Up @@ -114,31 +114,27 @@ def paginate_queryset(self, queryset, request, view=None):
"""
Paginate the Elasticsearch queryset using search_after.
"""

search_after = request.query_params.get(self.search_after_param)
if search_after:
try:
queryset = queryset.extra(search_after=json.loads(search_after))
except json.JSONDecodeError as exc:
raise ValueError("Invalid JSON format for search_after parameter") from exc

return super().paginate_queryset(queryset, request, view)
queryset = super().paginate_queryset(queryset, request, view)
self.last_obj = queryset[-1] if queryset else None # pylint: disable=attribute-defined-outside-init
return queryset

def get_next_link(self):
if not self.page.has_next():
return None

last_item_sort = self._get_last_item_sort()
last_item_sort = self.last_obj.meta.sort.copy() if hasattr(self, 'last_obj') and self.last_obj else None
if not last_item_sort:
return None

url = self.request.build_absolute_uri()
return replace_query_param(url, self.search_after_param, json.dumps(last_item_sort))

def _get_last_item_sort(self):
last_item = self.page.object_list[-1] if self.page.object_list else None
return list(last_item.meta.sort) if last_item else None


class BaseElasticsearchDocumentViewSet(mixins.DetailMixin, mixins.FacetMixin, DocumentViewSet):
lookup_field = 'key'
Expand Down

0 comments on commit 0ab4e42

Please sign in to comment.