diff --git a/lms/djangoapps/branding/__init__.py b/lms/djangoapps/branding/__init__.py index b7ce3e1df5..0847f631cb 100644 --- a/lms/djangoapps/branding/__init__.py +++ b/lms/djangoapps/branding/__init__.py @@ -14,7 +14,7 @@ from opaque_keys.edx.keys import CourseKey from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -def get_visible_courses(org=None, filter_=None): +def get_visible_courses(org=None, filter_=None, active_only=False): """ Yield the CourseOverviews that should be visible in this branded instance. @@ -24,6 +24,7 @@ def get_visible_courses(org=None, filter_=None): filtering by organization. filter_ (dict): Optional parameter that allows custom filtering by fields on the course. + active_only (bool): Optional parameter that enables fetching active courses only. """ # Import is placed here to avoid model import at project startup. from openedx.core.djangoapps.content.course_overviews.models import CourseOverview @@ -35,12 +36,12 @@ def get_visible_courses(org=None, filter_=None): if org: # Check the current site's orgs to make sure the org's courses should be displayed if not current_site_orgs or org in current_site_orgs: - courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_) + courses = CourseOverview.get_all_courses(orgs=[org], filter_=filter_, active_only=active_only) elif current_site_orgs: # Only display courses that should be displayed on this site - courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_) + courses = CourseOverview.get_all_courses(orgs=current_site_orgs, filter_=filter_, active_only=active_only) else: - courses = CourseOverview.get_all_courses(filter_=filter_) + courses = CourseOverview.get_all_courses(filter_=filter_, active_only=active_only) courses = courses.order_by('id') diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py index bf3520031b..721c213992 100644 --- a/lms/djangoapps/course_api/api.py +++ b/lms/djangoapps/course_api/api.py @@ -115,7 +115,8 @@ def list_courses(request, org=None, filter_=None, search_term=None, - permissions=None): + permissions=None, + active_only=False): """ Yield all available courses. @@ -144,12 +145,13 @@ def list_courses(request, permissions (list[str]): If specified, it filters visible `CourseOverview` objects by checking if each permission specified is granted for the username. + active_only (bool): Optional parameter that enables fetching active courses only. Return value: Yield `CourseOverview` objects representing the collection of courses. """ user = get_effective_user(request.user, username) - course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions) + course_qs = get_courses(user, org=org, filter_=filter_, permissions=permissions, active_only=active_only) course_qs = _filter_by_search(course_qs, search_term) return course_qs diff --git a/lms/djangoapps/course_api/forms.py b/lms/djangoapps/course_api/forms.py index 94fb34fde6..055692da71 100644 --- a/lms/djangoapps/course_api/forms.py +++ b/lms/djangoapps/course_api/forms.py @@ -62,6 +62,7 @@ class CourseListGetForm(UsernameValidatorMixin, Form): filter_type(param_name='mobile', field_name='mobile_available'), ] mobile = ExtendedNullBooleanField(required=False) + active_only = ExtendedNullBooleanField(required=False) permissions = MultiValueField(required=False) def clean(self): diff --git a/lms/djangoapps/course_api/tests/mixins.py b/lms/djangoapps/course_api/tests/mixins.py index eb1cef2612..2434443e87 100644 --- a/lms/djangoapps/course_api/tests/mixins.py +++ b/lms/djangoapps/course_api/tests/mixins.py @@ -17,13 +17,12 @@ class CourseApiFactoryMixin: """ @staticmethod - def create_course(**kwargs): + def create_course(end=datetime(2015, 9, 19, 18, 0, 0), **kwargs): """ Create a course for use in test cases """ - return ToyCourseFactory.create( - end=datetime(2015, 9, 19, 18, 0, 0), + end=end, enrollment_start=datetime(2015, 6, 15, 0, 0, 0), enrollment_end=datetime(2015, 7, 15, 0, 0, 0), emit_signals=True, diff --git a/lms/djangoapps/course_api/tests/test_forms.py b/lms/djangoapps/course_api/tests/test_forms.py index 76c2fd0812..9d14e5a998 100644 --- a/lms/djangoapps/course_api/tests/test_forms.py +++ b/lms/djangoapps/course_api/tests/test_forms.py @@ -70,6 +70,7 @@ class TestCourseListGetForm(FormTestMixin, UsernameTestMixin, SharedModuleStoreT 'search_term': '', 'filter_': None, 'permissions': set(), + 'active_only': None, } def test_basic(self): diff --git a/lms/djangoapps/course_api/tests/test_views.py b/lms/djangoapps/course_api/tests/test_views.py index eea5e4da07..57b64d264c 100644 --- a/lms/djangoapps/course_api/tests/test_views.py +++ b/lms/djangoapps/course_api/tests/test_views.py @@ -2,7 +2,7 @@ Tests for Course API views. """ -from datetime import datetime +from datetime import datetime, timedelta from hashlib import md5 from unittest import TestCase @@ -187,6 +187,21 @@ class CourseListViewTestCaseMultipleCourses(CourseApiTestViewMixin, ModuleStoreT filtered_response = self.verify_response(params={'org': self.course.org, 'username': self.staff_user.username}) assert all((course['org'] == self.course.org) for course in filtered_response.data['results']) + def test_filter_active_courses_only(self): + """ + Verify that CourseOverviews are filtered by end date if active_only filter is provided. + """ + self.setup_user(self.staff_user) + active_course = self.create_course(org='org1', end=datetime.now() + timedelta(days=1)) + missing_end_date = self.create_course(org='org2', end=None) + + response = self.verify_response(params={'username': self.staff_user.username, 'active_only': True}) + output_ids = {course['id'] for course in response.data['results']} + + assert len(output_ids) == 2 + assert str(self.course.id) not in output_ids + assert {str(active_course.id), str(missing_end_date.id)} == output_ids + def test_filter(self): self.setup_user(self.staff_user) diff --git a/lms/djangoapps/course_api/views.py b/lms/djangoapps/course_api/views.py index 5e4239e6f1..cee7d7cbb9 100644 --- a/lms/djangoapps/course_api/views.py +++ b/lms/djangoapps/course_api/views.py @@ -277,6 +277,11 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView): Notice that Staff users are always granted permission to list any course. + active_only (optional): + If this boolean is specified, only the courses that have not ended or do not have any end + date are returned. This is different from search_term because this filtering is done on + CourseOverview and not ElasticSearch. + **Returns** * 200 on success, with a list of course discovery objects as returned @@ -333,7 +338,8 @@ class CourseListView(DeveloperErrorViewMixin, ListAPIView): org=form.cleaned_data['org'], filter_=form.cleaned_data['filter_'], search_term=form.cleaned_data['search_term'], - permissions=form.cleaned_data['permissions'] + permissions=form.cleaned_data['permissions'], + active_only=form.cleaned_data.get('active_only', False) ) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 411acc5515..e390e3b43e 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -753,7 +753,7 @@ def get_course_syllabus_section(course, section_key): @function_trace('get_courses') -def get_courses(user, org=None, filter_=None, permissions=None): +def get_courses(user, org=None, filter_=None, permissions=None, active_only=False): """ Return a LazySequence of courses available, optionally filtered by org code (case-insensitive) or a set of permissions to be satisfied for the specified @@ -763,6 +763,7 @@ def get_courses(user, org=None, filter_=None, permissions=None): courses = branding.get_visible_courses( org=org, filter_=filter_, + active_only=active_only ).prefetch_related( 'modes', ).select_related( diff --git a/openedx/core/djangoapps/content/course_overviews/models.py b/openedx/core/djangoapps/content/course_overviews/models.py index 14eb3095a6..55d2b8d2bb 100644 --- a/openedx/core/djangoapps/content/course_overviews/models.py +++ b/openedx/core/djangoapps/content/course_overviews/models.py @@ -5,8 +5,10 @@ Declaration of CourseOverview model import json import logging +from datetime import datetime from urllib.parse import urlparse, urlunparse +import pytz from ccx_keys.locator import CCXLocator from config_models.models import ConfigurationModel from django.conf import settings @@ -655,7 +657,7 @@ class CourseOverview(TimeStampedModel): log.info('Finished generating course overviews.') @classmethod - def get_all_courses(cls, orgs=None, filter_=None): + def get_all_courses(cls, orgs=None, filter_=None, active_only=False): """ Return a queryset containing all CourseOverview objects in the database. @@ -663,6 +665,7 @@ class CourseOverview(TimeStampedModel): orgs (list[string]): Optional parameter that allows case-insensitive filtering by organization. filter_ (dict): Optional parameter that allows custom filtering. + active_only (bool): If provided, only the courses that have not ended will be returned. """ # Note: If a newly created course is not returned in this QueryList, # make sure the "publish" signal was emitted when the course was @@ -680,6 +683,10 @@ class CourseOverview(TimeStampedModel): if filter_: course_overviews = course_overviews.filter(**filter_) + if active_only: + course_overviews = course_overviews.filter( + Q(end__isnull=True) | Q(end__gte=datetime.now().replace(tzinfo=pytz.UTC)) + ) return course_overviews diff --git a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py index 690db4834d..6e69c3dd04 100644 --- a/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py +++ b/openedx/core/djangoapps/content/course_overviews/tests/test_course_overviews.py @@ -554,6 +554,20 @@ class CourseOverviewTestCase(CatalogIntegrationMixin, ModuleStoreTestCase, Cache assert {course_overview.id for course_overview in CourseOverview.get_all_courses(filter_=filter_)} ==\ expected_courses, f'testing CourseOverview.get_all_courses with filter_={filter_}' + def test_get_all_active_courses(self): + """ + Verify active courses or courses with null end date are returned if active_only is provided. + """ + active_course = CourseFactory.create(emit_signals=True, end=self.DATES[self.NEXT_MONTH]) + missing_end_date = CourseFactory.create(emit_signals=True, end=None) + inactive_course = CourseFactory.create(emit_signals=True, end=self.DATES[self.LAST_MONTH]) + + output_ids = {course.id for course in CourseOverview.get_all_courses(active_only=True)} + + assert len(output_ids) == 2 + assert inactive_course.id not in output_ids + assert {active_course.id, missing_end_date.id} == output_ids + def test_get_from_ids(self): """ Assert that CourseOverviews.get_from_ids works as expected.