fix: [FC-0031] Add limit the number of returned results for mobile_search
This commit is contained in:
committed by
Glib Glugovskiy
parent
763b102ac9
commit
258f3fc8a0
@@ -82,7 +82,7 @@ def course_detail(request, username, course_key):
|
||||
return overview
|
||||
|
||||
|
||||
def _filter_by_search(course_queryset, search_term):
|
||||
def _filter_by_search(course_queryset, search_term, mobile_search=False):
|
||||
"""
|
||||
Filters a course queryset by the specified search term.
|
||||
"""
|
||||
@@ -100,10 +100,20 @@ def _filter_by_search(course_queryset, search_term):
|
||||
)
|
||||
|
||||
search_courses_ids = {course['data']['id'] for course in search_courses['results']}
|
||||
courses = [course for course in course_queryset if str(course.id) in search_courses_ids]
|
||||
|
||||
if mobile_search is True:
|
||||
course_limit = getattr(settings, 'MOBILE_SEARCH_COURSE_LIMIT', 100)
|
||||
courses = [course for course in course_queryset[:course_limit] if str(course.id) in search_courses_ids]
|
||||
return LazySequence(
|
||||
iter(courses),
|
||||
est_len=len(courses)
|
||||
)
|
||||
return LazySequence(
|
||||
iter(courses),
|
||||
est_len=len(courses)
|
||||
(
|
||||
course for course in course_queryset
|
||||
if str(course.id) in search_courses_ids
|
||||
),
|
||||
est_len=len(course_queryset)
|
||||
)
|
||||
|
||||
|
||||
@@ -114,7 +124,8 @@ def list_courses(request,
|
||||
search_term=None,
|
||||
permissions=None,
|
||||
active_only=False,
|
||||
course_keys=None):
|
||||
course_keys=None,
|
||||
mobile_search=False):
|
||||
"""
|
||||
Yield all available courses.
|
||||
|
||||
@@ -147,6 +158,9 @@ def list_courses(request,
|
||||
course_keys (list[str]):
|
||||
If specified, it filters visible `CourseOverview` objects by
|
||||
the course keys (ids) provided
|
||||
mobile_search (bool):
|
||||
Optional parameter that limits the number of returned courses
|
||||
to MOBILE_SEARCH_COURSE_LIMIT.
|
||||
|
||||
Return value:
|
||||
Yield `CourseOverview` objects representing the collection of courses.
|
||||
@@ -155,7 +169,7 @@ def list_courses(request,
|
||||
course_qs = get_courses(
|
||||
user, org=org, filter_=filter_, permissions=permissions, active_only=active_only, course_keys=course_keys
|
||||
)
|
||||
course_qs = _filter_by_search(course_qs, search_term)
|
||||
course_qs = _filter_by_search(course_qs, search_term, mobile_search)
|
||||
return course_qs
|
||||
|
||||
|
||||
|
||||
@@ -65,6 +65,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form):
|
||||
active_only = ExtendedNullBooleanField(required=False)
|
||||
permissions = MultiValueField(required=False)
|
||||
course_keys = MultiValueField(required=False)
|
||||
mobile_search = ExtendedNullBooleanField(required=False)
|
||||
|
||||
def clean(self):
|
||||
"""
|
||||
|
||||
@@ -72,6 +72,7 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT
|
||||
'permissions': set(),
|
||||
'active_only': None,
|
||||
'course_keys': set(),
|
||||
'mobile_search': None,
|
||||
}
|
||||
|
||||
def test_basic(self):
|
||||
|
||||
@@ -454,10 +454,12 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
Test count items in pagination for api courses list - class CourseListView
|
||||
"""
|
||||
# Create 15 new courses, courses have the word "new" in the title
|
||||
_ = [self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)]
|
||||
[self.create_and_index_course(f"numb_{number}", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
|
||||
response = self.verify_response(params={"search_term": "new"})
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.json()["pagination"]["count"], 15)
|
||||
# We don't have 'count' 15 because 'mobile_search' param is None
|
||||
# And LazySequence contains all courses
|
||||
self.assertEqual(response.json()["pagination"]["count"], 18)
|
||||
|
||||
def test_count_item_pagination_with_search_term_and_filter(self):
|
||||
"""
|
||||
@@ -466,12 +468,27 @@ class CourseListSearchViewTest(CourseApiTestViewMixin, ModuleStoreTestCase, Sear
|
||||
class CourseListView
|
||||
"""
|
||||
# Create 25 new courses with two different organisations
|
||||
_ = [self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)]
|
||||
_ = [self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)]
|
||||
[self.create_and_index_course("Org_N", f"new_{number}") for number in range(10)] # pylint: disable=expression-not-assigned
|
||||
[self.create_and_index_course("Org_X", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
|
||||
response = self.verify_response(params={"org": "Org_X", "search_term": "new"})
|
||||
self.assertEqual(response.status_code, 200)
|
||||
self.assertEqual(response.json()["pagination"]["count"], 15)
|
||||
|
||||
def test_count_item_pagination_with_search_term_and_mobile_search(self):
|
||||
"""
|
||||
Test count items in pagination for api courses list
|
||||
with search_term and 'mobile_search' is True
|
||||
"""
|
||||
# Create 25 new courses with two different words in titles
|
||||
[self.create_and_index_course("Org_N", f"old_{number}") for number in range(10)] # pylint: disable=expression-not-assigned
|
||||
[self.create_and_index_course("Org_N", f"new_{number}") for number in range(15)] # pylint: disable=expression-not-assigned
|
||||
response = self.verify_response(
|
||||
params={"search_term": "new", "mobile_search": True}
|
||||
)
|
||||
self.assertEqual(response.status_code, 200)
|
||||
# We have 'count' 15 because 'mobile_search' param is true
|
||||
self.assertEqual(response.json()["pagination"]["count"], 15)
|
||||
|
||||
|
||||
class CourseIdListViewTestCase(CourseApiTestViewMixin, ModuleStoreTestCase):
|
||||
"""
|
||||
|
||||
@@ -290,6 +290,10 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
If specified, it fetches the `CourseOverview` objects for the
|
||||
the specified course keys
|
||||
|
||||
mobile_search (bool):
|
||||
Optional parameter that limits the number of returned courses
|
||||
to MOBILE_SEARCH_COURSE_LIMIT.
|
||||
|
||||
**Returns**
|
||||
|
||||
* 200 on success, with a list of course discovery objects as returned
|
||||
@@ -349,6 +353,7 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView):
|
||||
permissions=form.cleaned_data['permissions'],
|
||||
active_only=form.cleaned_data.get('active_only', False),
|
||||
course_keys=form.cleaned_data['course_keys'],
|
||||
mobile_search=form.cleaned_data.get('mobile_search', False),
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -4428,6 +4428,9 @@ MOBILE_APP_USER_AGENT_REGEXES = [
|
||||
r'edX/org.edx.mobile',
|
||||
]
|
||||
|
||||
# set course limit for mobile search
|
||||
MOBILE_SEARCH_COURSE_LIMIT = 100
|
||||
|
||||
# cache timeout in seconds for Mobile App Version Upgrade
|
||||
APP_UPGRADE_CACHE_TIMEOUT = 3600
|
||||
|
||||
|
||||
Reference in New Issue
Block a user