Merge pull request #62 from edx/clintonb/catalog-filter

Filtering catalog courses

course_discovery/apps/api/serializers.py

@@ -129,6 +129,10 @@ class CourseSerializer(TimestampModelSerializer):
         )
 
 
+class CourseSerializerExcludingClosedRuns(CourseSerializer):
+    course_runs = CourseRunSerializer(many=True, source='active_course_runs')
+
+
 class ContainedCoursesSerializer(serializers.Serializer):  # pylint: disable=abstract-method
     courses = serializers.DictField(
         child=serializers.BooleanField(),

course_discovery/apps/api/v1/tests/test_views/mixins.py

@@ -6,7 +6,9 @@ import responses
 from django.conf import settings
 from rest_framework.test import APIRequestFactory
 
-from course_discovery.apps.api.serializers import CatalogSerializer, CourseSerializer
+from course_discovery.apps.api.serializers import (
+    CatalogSerializer, CourseSerializer, CourseSerializerExcludingClosedRuns
+)
 
 
 class SerializationMixin(object):
@@ -25,6 +27,9 @@ class SerializationMixin(object):
     def serialize_course(self, course, many=False, format=None):
         return self._serialize_object(CourseSerializer, course, many, format)
 
+    def serialize_catalog_course(self, course, many=False, format=None):
+        return self._serialize_object(CourseSerializerExcludingClosedRuns, course, many, format)
+
 
 class OAuth2Mixin(object):
     def generate_oauth2_token_header(self, user):

course_discovery/apps/api/v1/tests/test_views/test_catalogs.py

 # pylint: disable=redefined-builtin,no-member
+import datetime
 import urllib
 
 import ddt
+import pytz
 import responses
 from rest_framework.reverse import reverse
 from rest_framework.test import APITestCase
@@ -12,7 +14,7 @@ from course_discovery.apps.catalogs.models import Catalog
 from course_discovery.apps.catalogs.tests.factories import CatalogFactory
 from course_discovery.apps.core.tests.factories import UserFactory
 from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
-from course_discovery.apps.course_metadata.tests.factories import CourseFactory
+from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory
 
 
 @ddt.ddt
@@ -24,7 +26,9 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
         self.user = UserFactory(is_staff=True, is_superuser=True)
         self.client.force_authenticate(self.user)
         self.catalog = CatalogFactory(query='title:abc*')
-        self.course = CourseFactory(key='a/b/c', title='ABC Test Course')
+        enrollment_end = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30)
+        self.course_run = CourseRunFactory(enrollment_end=enrollment_end, course__title='ABC Test Course')
+        self.course = self.course_run.course
         self.refresh_index()
 
     def assert_catalog_created(self, **headers):
@@ -87,9 +91,14 @@ class CatalogViewSetTests(ElasticsearchTestMixin, SerializationMixin, OAuth2Mixi
         url = reverse('api:v1:catalog-courses', kwargs={'id': self.catalog.id})
         courses = [self.course]
 
+        # These courses/course runs should not be returned because they are no longer open for enrollment.
+        enrollment_end = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=30)
+        CourseRunFactory(enrollment_end=enrollment_end, course__title='ABC Test Course 2')
+        CourseRunFactory(enrollment_end=enrollment_end, course=self.course)
+
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
-        self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True))
+        self.assertListEqual(response.data['results'], self.serialize_catalog_course(courses, many=True))
 
     def test_contains(self):
         """ Verify the endpoint returns a filtered list of courses contained in the catalog. """

course_discovery/apps/api/v1/views.py

@@ -8,8 +8,9 @@ from rest_framework.permissions import IsAuthenticated
 from rest_framework.response import Response
 
 from course_discovery.apps.api.filters import PermissionsFilter
-from course_discovery.apps.api.serializers import(
-    CatalogSerializer, CourseSerializer, CourseRunSerializer, ContainedCoursesSerializer
+from course_discovery.apps.api.serializers import (
+    CatalogSerializer, CourseSerializer, CourseRunSerializer, ContainedCoursesSerializer,
+    CourseSerializerExcludingClosedRuns,
 )
 from course_discovery.apps.catalogs.models import Catalog
 from course_discovery.apps.course_metadata.constants import COURSE_ID_REGEX, COURSE_RUN_ID_REGEX
@@ -57,15 +58,18 @@ class CatalogViewSet(viewsets.ModelViewSet):
     def courses(self, request, id=None):  # pylint: disable=redefined-builtin,unused-argument
         """
         Retrieve the list of courses contained within this catalog.
+
+        Only courses with active course runs are returned. A course run is considered active if it is currently
+        open for enrollment, or will open in the future.
         ---
-        serializer: CourseSerializer
+        serializer: CourseSerializerExcludingClosedRuns
         """
 
         catalog = self.get_object()
-        queryset = catalog.courses()
+        queryset = catalog.courses().active()
 
         page = self.paginate_queryset(queryset)
-        serializer = CourseSerializer(page, many=True, context={'request': request})
+        serializer = CourseSerializerExcludingClosedRuns(page, many=True, context={'request': request})
         return self.get_paginated_response(serializer.data)
 
     @detail_route()

course_discovery/apps/catalogs/models.py

@@ -27,10 +27,11 @@ class Catalog(ModelPermissionsMixin, TimeStampedModel):
         """ Returns the list of courses contained within this catalog.
 
         Returns:
-            Course[]
+            QuerySet
         """
-        results = self._get_query_results().load_all()
-        return [result.object for result in results]
+        results = self._get_query_results()
+        ids = [result.pk for result in results]
+        return Course.objects.filter(pk__in=ids)
 
     @property
     def courses_count(self):

course_discovery/apps/catalogs/tests/test_models.py

@@ -24,8 +24,8 @@ class CatalogTests(ElasticsearchTestMixin, TestCase):
         self.assertEqual(str(self.catalog), expected)
 
     def test_courses(self):
-        """ Verify the method returns a list of courses contained in the catalog. """
-        self.assertEqual(self.catalog.courses(), [self.course])
+        """ Verify the method returns a QuerySet of courses contained in the catalog. """
+        self.assertEqual(list(self.catalog.courses()), [self.course])
 
     def test_contains(self):
         """ Verify the method returns a mapping of course IDs to booleans. """

course_discovery/apps/course_metadata/models.py

+import datetime
 import logging
 
+import pytz
 from django.db import models
 from django.utils.translation import ugettext_lazy as _
 from django_extensions.db.models import TimeStampedModel
@@ -7,6 +9,7 @@ from simple_history.models import HistoricalRecords
 from sortedm2m.fields import SortedManyToManyField
 
 from course_discovery.apps.core.models import Currency
+from course_discovery.apps.course_metadata.query import CourseQuerySet
 from course_discovery.apps.ietf_language_tags.models import LanguageTag
 
 logger = logging.getLogger(__name__)
@@ -130,6 +133,7 @@ class Course(TimeStampedModel):
     marketing_url = models.URLField(max_length=255, null=True, blank=True)
 
     history = HistoricalRecords()
+    objects = CourseQuerySet.as_manager()
 
     @property
     def owners(self):
@@ -139,6 +143,15 @@ class Course(TimeStampedModel):
     def sponsors(self):
         return self.organizations.filter(courseorganization__relation_type=CourseOrganization.SPONSOR)
 
+    @property
+    def active_course_runs(self):
+        """ Returns course runs currently open for enrollment, or opening in the future.
+
+        Returns:
+            QuerySet
+        """
+        return self.course_runs.filter(enrollment_end__gt=datetime.datetime.now(pytz.UTC))
+
     def __str__(self):
         return '{key}: {title}'.format(key=self.key, title=self.title)
 

course_discovery/apps/course_metadata/query.py

+import datetime
+
+import pytz
+from django.db import models
+
+
+class CourseQuerySet(models.QuerySet):
+    def active(self):
+        """ Filters Courses to those with CourseRuns that are either currently open for enrollment,
+        or will be open for enrollment in the future. """
+
+        return self.filter(course_runs__enrollment_end__gt=datetime.datetime.now(pytz.UTC))

course_discovery/apps/course_metadata/tests/test_models.py

+import datetime
+
 import ddt
+import pytz
 from django.test import TestCase
 
-from course_discovery.apps.course_metadata.models import(
+from course_discovery.apps.course_metadata.models import (
     AbstractNamedModel, AbstractMediaModel, AbstractValueModel, CourseOrganization
 )
 from course_discovery.apps.course_metadata.tests import factories
@@ -42,6 +45,19 @@ class CourseTests(TestCase):
         self.assertEqual(len(sponsors), 1)
         self.assertEqual(sponsors[0], self.sponsor)
 
+    def test_active_course_runs(self):
+        """ Verify the property returns only course runs currently open for enrollment or opening in the future. """
+        # pylint: disable=no-member
+        self.assertListEqual(list(self.course.active_course_runs), [])
+
+        enrollment_end = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=1)
+        factories.CourseRunFactory(course=self.course, enrollment_end=enrollment_end)
+        self.assertListEqual(list(self.course.active_course_runs), [])
+
+        enrollment_end = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=1)
+        active = factories.CourseRunFactory(course=self.course, enrollment_end=enrollment_end)
+        self.assertListEqual(list(self.course.active_course_runs), [active])
+
 
 @ddt.ddt
 class CourseRunTests(TestCase):

course_discovery/apps/course_metadata/tests/test_query.py

+import datetime
+
+import pytz
+from django.test import TestCase
+
+from course_discovery.apps.course_metadata.models import Course
+from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory
+
+
+class CourseQuerySetTests(TestCase):
+    def test_active(self):
+        """ Verify the method filters the Courses to those with active course runs. """
+        # Create an active course
+        enrollment_end = datetime.datetime.now(pytz.UTC) + datetime.timedelta(days=30)
+        active_course = CourseRunFactory(enrollment_end=enrollment_end).course
+
+        # Create an inactive course
+        enrollment_end = datetime.datetime.now(pytz.UTC) - datetime.timedelta(days=30)
+        CourseRunFactory(enrollment_end=enrollment_end, course__title='ABC Test Course 2')
+
+        self.assertListEqual(list(Course.objects.active()), [active_course])