Merge pull request #307 from edx/renzo/program-course-ordering

Order courses within a serialized program

course_discovery/apps/api/serializers.py

 # pylint: disable=abstract-method
+from datetime import datetime
 import json
 from urllib.parse import urlencode
 
 from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext_lazy as _
 from drf_haystack.serializers import HaystackSerializer, HaystackFacetSerializer
+import pytz
 from rest_framework import serializers
 from rest_framework.fields import DictField
 from taggit_serializer.serializers import TagListSerializerField, TaggitSerializer
@@ -380,16 +382,56 @@ class ProgramSerializer(serializers.ModelSerializer):
     staff = PersonSerializer(many=True)
 
     def get_courses(self, program):
+        courses = self.sort_courses(program)
+
         course_serializer = ProgramCourseSerializer(
-            program.courses,
+            courses,
             many=True,
             context={
                 'request': self.context.get('request'),
                 'program': program
             }
         )
+
         return course_serializer.data
 
+    def sort_courses(self, program):
+        """
+        Sorting by enrollment start then by course start yields a list ordered by course start, with
+        ties broken by enrollment start. This works because Python sorting is stable: two objects with
+        equal keys appear in the same order in sorted output as they appear in the input.
+
+        Courses are only created if there's at least one course run belonging to that course, so
+        course_runs should never be empty. If it is, key functions in this method attempting to find the
+        min of an empty sequence will raise a ValueError.
+        """
+        def min_run_enrollment_start(course):
+            # Enrollment starts may be empty. When this is the case, we make the same assumption as
+            # the LMS: no enrollment_start is equivalent to (offset-aware) datetime.min.
+            min_datetime = datetime.min.replace(tzinfo=pytz.UTC)
+
+            # Course runs excluded from the program are excluded here, too.
+            #
+            # If this becomes a candidate for optimization in the future, be careful sorting null values
+            # in the database. PostgreSQL and MySQL sort null values as if they are higher than non-null
+            # values, while SQLite does the opposite.
+            #
+            # For more, refer to https://docs.djangoproject.com/en/1.10/ref/models/querysets/#latest.
+            run = min(program.course_runs.filter(course=course), key=lambda run: run.enrollment_start or min_datetime)
+
+            return run.enrollment_start or min_datetime
+
+        def min_run_start(course):
+            run = min(program.course_runs.filter(course=course), key=lambda run: run.start)
+
+            return run.start
+
+        courses = list(program.courses.all())
+        courses.sort(key=min_run_enrollment_start)
+        courses.sort(key=min_run_start)
+
+        return courses
+
     class Meta:
         model = Program
         fields = (

course_discovery/apps/api/tests/test_serializers.py

@@ -229,6 +229,13 @@ class ProgramSerializerTests(TestCase):
         request = make_request()
         org_list = OrganizationFactory.create_batch(1)
         course_list = CourseFactory.create_batch(3)
+        for course in course_list:
+            CourseRunFactory.create_batch(
+                3,
+                course=course,
+                enrollment_start=datetime(2014, 1, 1),
+                start=datetime(2014, 1, 1)
+            )
         corporate_endorsements = CorporateEndorsementFactory.create_batch(1)
         individual_endorsements = EndorsementFactory.create_batch(1)
         staff = PersonFactory.create_batch(1)
@@ -310,7 +317,12 @@ class ProgramSerializerTests(TestCase):
         course_list = CourseFactory.create_batch(4)
         excluded_runs = []
         for course in course_list:
-            course_runs = CourseRunFactory.create_batch(3, course=course)
+            course_runs = CourseRunFactory.create_batch(
+                3,
+                course=course,
+                enrollment_start=datetime(2014, 1, 1),
+                start=datetime(2014, 1, 1)
+            )
             excluded_runs.append(course_runs[0])
 
         program = ProgramFactory(
@@ -361,6 +373,96 @@ class ProgramSerializerTests(TestCase):
 
         self.assertDictEqual(serializer.data, expected)
 
+    def test_course_ordering(self):
+        """
+        Verify that courses in a program are ordered by ascending run start date,
+        with ties broken by earliest run enrollment start date.
+        """
+        request = make_request()
+        course_list = CourseFactory.create_batch(3)
+
+        # Create a course run with arbitrary start and empty enrollment_start.
+        CourseRunFactory(
+            course=course_list[2],
+            enrollment_start=None,
+            start=datetime(2014, 2, 1),
+        )
+
+        # Create a second run with matching start, but later enrollment_start.
+        CourseRunFactory(
+            course=course_list[1],
+            enrollment_start=datetime(2014, 1, 2),
+            start=datetime(2014, 2, 1),
+        )
+
+        # Create a third run with later start and enrollment_start.
+        CourseRunFactory(
+            course=course_list[0],
+            enrollment_start=datetime(2014, 2, 1),
+            start=datetime(2014, 3, 1),
+        )
+
+        program = ProgramFactory(courses=course_list)
+        serializer = ProgramSerializer(program, context={'request': request})
+
+        expected = ProgramCourseSerializer(
+            # The expected ordering is the reverse of course_list.
+            course_list[::-1],
+            many=True,
+            context={'request': request, 'program': program}
+        ).data
+
+        self.assertEqual(serializer.data['courses'], expected)
+
+    def test_course_ordering_with_exclusions(self):
+        """
+        Verify that excluded course runs aren't used when ordering courses.
+        """
+        request = make_request()
+        course_list = CourseFactory.create_batch(3)
+
+        # Create a course run with arbitrary start and empty enrollment_start.
+        # This run will be excluded from the program. If it wasn't excluded,
+        # the expected course ordering, by index, would be: 0, 2, 1.
+        excluded_run = CourseRunFactory(
+            course=course_list[0],
+            enrollment_start=None,
+            start=datetime(2014, 1, 1),
+        )
+
+        # Create a run with later start and empty enrollment_start.
+        CourseRunFactory(
+            course=course_list[2],
+            enrollment_start=None,
+            start=datetime(2014, 2, 1),
+        )
+
+        # Create a run with matching start, but later enrollment_start.
+        CourseRunFactory(
+            course=course_list[1],
+            enrollment_start=datetime(2014, 1, 2),
+            start=datetime(2014, 2, 1),
+        )
+
+        # Create a run with later start and enrollment_start.
+        CourseRunFactory(
+            course=course_list[0],
+            enrollment_start=datetime(2014, 2, 1),
+            start=datetime(2014, 3, 1),
+        )
+
+        program = ProgramFactory(courses=course_list, excluded_course_runs=[excluded_run])
+        serializer = ProgramSerializer(program, context={'request': request})
+
+        expected = ProgramCourseSerializer(
+            # The expected ordering is the reverse of course_list.
+            course_list[::-1],
+            many=True,
+            context={'request': request, 'program': program}
+        ).data
+
+        self.assertEqual(serializer.data['courses'], expected)
+
 
 class ContainedCourseRunsSerializerTests(TestCase):
     def test_data(self):