Skip to content

Commit

Permalink
Merge pull request #381 from edx/schen/ECOM-5729
Browse files Browse the repository at this point in the history
Exclude deleted programs from course and course_run api
  • Loading branch information
schenedx authored Oct 17, 2016
2 parents 823f2de + eb4070f commit 910299d
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 22 deletions.
17 changes: 14 additions & 3 deletions course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

from course_discovery.apps.api.fields import StdImageSerializerField, ImageField
from course_discovery.apps.catalogs.models import Catalog
from course_discovery.apps.course_metadata.choices import CourseRunStatus
from course_discovery.apps.course_metadata.choices import CourseRunStatus, ProgramStatus
from course_discovery.apps.course_metadata.models import (
Course, CourseRun, Image, Organization, Person, Prerequisite, Seat, Subject, Video, Program, ProgramType, FAQ,
CorporateEndorsement, Endorsement, Position
Expand Down Expand Up @@ -384,8 +384,11 @@ class CourseRunWithProgramsSerializer(CourseRunSerializer):
programs = serializers.SerializerMethodField()

def get_programs(self, obj):
# Filter out non-deleted programs which this course_run is part of the program course_run exclusion
programs = [program for program in obj.programs.all()
if obj.id not in (run.id for run in program.excluded_course_runs.all())]
if (self.context.get('include_deleted_programs') or
program.status != ProgramStatus.Deleted) and
obj.id not in (run.id for run in program.excluded_course_runs.all())]

return NestedProgramSerializer(programs, many=True).data

Expand Down Expand Up @@ -459,7 +462,15 @@ def get_marketing_url(self, obj):

class CourseWithProgramsSerializer(CourseSerializer):
"""A ``CourseSerializer`` which includes programs."""
programs = NestedProgramSerializer(many=True)
programs = serializers.SerializerMethodField()

def get_programs(self, obj):
if self.context.get('include_deleted_programs'):
eligible_programs = obj.programs.all()
else:
eligible_programs = obj.programs.exclude(status=ProgramStatus.Deleted)

return NestedProgramSerializer(eligible_programs, many=True).data

class Meta(CourseSerializer.Meta):
model = Course
Expand Down
86 changes: 72 additions & 14 deletions course_discovery/apps/api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,43 @@ class CourseWithProgramsSerializerTests(CourseSerializerTests):
def get_expected_data(self, course, request):
expected = super().get_expected_data(course, request)
expected.update({
'programs': NestedProgramSerializer(course.programs, many=True, context={'request': request}).data,
'programs': NestedProgramSerializer(
course.programs,
many=True,
context={'request': request}
).data,
})

return expected

def setUp(self):
super().setUp()
self.request = make_request()
self.course = CourseFactory()
self.deleted_program = ProgramFactory(
courses=[self.course],
status=ProgramStatus.Deleted
)

def test_exclude_deleted_programs(self):
"""
If the associated program is deleted,
CourseWithProgramsSerializer should not return any serialized programs
"""
serializer = self.serializer_class(self.course, context={'request': self.request})
self.assertEqual(serializer.data['programs'], [])

def test_include_deleted_programs(self):
"""
If the associated program is deleted, but we are sending in the 'include_deleted_programs' flag
CourseWithProgramsSerializer should return deleted programs
"""
serializer = self.serializer_class(
self.course,
context={'request': self.request, 'include_deleted_programs': 1}
)
self.assertEqual(serializer.data, self.get_expected_data(self.course, self.request))


class MinimalCourseRunSerializerTests(TestCase):
serializer_class = MinimalCourseRunSerializer
Expand Down Expand Up @@ -234,16 +266,23 @@ def test_exclude_utm(self):


class CourseRunWithProgramsSerializerTests(TestCase):
def setUp(self):
super().setUp()
self.request = make_request()
self.course_run = CourseRunFactory()
self.serializer_context = {'request': self.request}

def test_data(self):
request = make_request()
course_run = CourseRunFactory()
serializer_context = {'request': request}
serializer = CourseRunWithProgramsSerializer(course_run, context=serializer_context)
ProgramFactory(courses=[course_run.course])
serializer = CourseRunWithProgramsSerializer(self.course_run, context=self.serializer_context)
ProgramFactory(courses=[self.course_run.course])

expected = CourseRunSerializer(course_run, context=serializer_context).data
expected = CourseRunSerializer(self.course_run, context=self.serializer_context).data
expected.update({
'programs': NestedProgramSerializer(course_run.course.programs, many=True, context=serializer_context).data,
'programs': NestedProgramSerializer(
self.course_run.course.programs,
many=True,
context=self.serializer_context
).data,
})

self.assertDictEqual(serializer.data, expected)
Expand All @@ -253,18 +292,37 @@ def test_data_excluded_course_run(self):
If a course run is excluded on a program, that program should not be
returned for that course run on the course run endpoint.
"""
request = make_request()
course_run = CourseRunFactory()
serializer_context = {'request': request}
serializer = CourseRunWithProgramsSerializer(course_run, context=serializer_context)
ProgramFactory(courses=[course_run.course], excluded_course_runs=[course_run])
expected = CourseRunSerializer(course_run, context=serializer_context).data
serializer = CourseRunWithProgramsSerializer(self.course_run, context=self.serializer_context)
ProgramFactory(courses=[self.course_run.course], excluded_course_runs=[self.course_run])
expected = CourseRunSerializer(self.course_run, context=self.serializer_context).data
expected.update({
'programs': [],
})

self.assertDictEqual(serializer.data, expected)

def test_exclude_deleted_programs(self):
"""
If the associated program is deleted,
CourseRunWithProgramsSerializer should not return any serialized programs
"""
ProgramFactory(courses=[self.course_run.course], status=ProgramStatus.Deleted)
serializer = CourseRunWithProgramsSerializer(self.course_run, context=self.serializer_context)
self.assertEqual(serializer.data['programs'], [])

def test_include_deleted_programs(self):
"""
If the associated program is deleted, but we are sending in the 'include_deleted_programs' flag
CourseRunWithProgramsSerializer should return deleted programs
"""
deleted_program = ProgramFactory(courses=[self.course_run.course], status=ProgramStatus.Deleted)
self.serializer_context['include_deleted_programs'] = 1
serializer = CourseRunWithProgramsSerializer(self.course_run, context=self.serializer_context)
self.assertEqual(
serializer.data['programs'],
NestedProgramSerializer([deleted_program], many=True, context=self.serializer_context).data
)


class FlattenedCourseRunWithCourseSerializerTests(TestCase): # pragma: no cover
def serialize_seats(self, course_run):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@
from course_discovery.apps.api.v1.tests.test_views.mixins import SerializationMixin
from course_discovery.apps.core.tests.factories import UserFactory
from course_discovery.apps.core.tests.mixins import ElasticsearchTestMixin
from course_discovery.apps.course_metadata.choices import ProgramStatus
from course_discovery.apps.course_metadata.models import CourseRun
from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory, PartnerFactory
from course_discovery.apps.course_metadata.tests.factories import CourseRunFactory, PartnerFactory, ProgramFactory


@ddt.ddt
Expand All @@ -38,6 +39,35 @@ def test_get(self):
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data, self.serialize_course_run(self.course_run))

def test_get_exclude_deleted_programs(self):
""" Verify the endpoint returns no associated deleted programs """
ProgramFactory(courses=[self.course_run.course], status=ProgramStatus.Deleted)

url = reverse('api:v1:course_run-detail', kwargs={'key': self.course_run.key})

with self.assertNumQueries(12):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data.get('programs'), [])

def test_get_include_deleted_programs(self):
"""
Verify the endpoint returns associated deleted programs
with the 'include_deleted_programs' flag set to True
"""
ProgramFactory(courses=[self.course_run.course], status=ProgramStatus.Deleted)

url = reverse('api:v1:course_run-detail', kwargs={'key': self.course_run.key})
url += '?include_deleted_programs=1'

with self.assertNumQueries(19):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(
response.data,
self.serialize_course_run(self.course_run, extra_context={'include_deleted_programs': True})
)

def test_list(self):
""" Verify the endpoint returns a list of all catalogs. """
url = reverse('api:v1:course_run-list')
Expand Down
34 changes: 30 additions & 4 deletions course_discovery/apps/api/v1/tests/test_views/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

from course_discovery.apps.api.v1.tests.test_views.mixins import SerializationMixin
from course_discovery.apps.core.tests.factories import UserFactory, USER_PASSWORD
from course_discovery.apps.course_metadata.choices import ProgramStatus
from course_discovery.apps.course_metadata.models import Course
from course_discovery.apps.course_metadata.tests.factories import CourseFactory
from course_discovery.apps.course_metadata.tests.factories import CourseFactory, ProgramFactory


class CourseViewSetTests(SerializationMixin, APITestCase):
Expand All @@ -21,16 +22,41 @@ def test_get(self):
""" Verify the endpoint returns the details for a single course. """
url = reverse('api:v1:course-detail', kwargs={'key': self.course.key})

with self.assertNumQueries(18):
with self.assertNumQueries(19):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data, self.serialize_course(self.course))

def test_get_exclude_deleted_programs(self):
""" Verify the endpoint returns no deleted associated programs """
ProgramFactory(courses=[self.course], status=ProgramStatus.Deleted)
url = reverse('api:v1:course-detail', kwargs={'key': self.course.key})
with self.assertNumQueries(12):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.data.get('programs'), [])

def test_get_include_deleted_programs(self):
"""
Verify the endpoint returns associated deleted programs
with the 'include_deleted_programs' flag set to True
"""
ProgramFactory(courses=[self.course], status=ProgramStatus.Deleted)
url = reverse('api:v1:course-detail', kwargs={'key': self.course.key})
url += '?include_deleted_programs=1'
with self.assertNumQueries(22):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertEqual(
response.data,
self.serialize_course(self.course, extra_context={'include_deleted_programs': True})
)

def test_list(self):
""" Verify the endpoint returns a list of all courses. """
url = reverse('api:v1:course-list')

with self.assertNumQueries(24):
with self.assertNumQueries(25):
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
self.assertListEqual(
Expand All @@ -57,7 +83,7 @@ def test_list_key_filter(self):
keys = ','.join([course.key for course in courses])
url = '{root}?keys={keys}'.format(root=reverse('api:v1:course-list'), keys=keys)

with self.assertNumQueries(35):
with self.assertNumQueries(38):
response = self.client.get(url)
self.assertListEqual(response.data['results'], self.serialize_course(courses, many=True))

Expand Down
14 changes: 14 additions & 0 deletions course_discovery/apps/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ def get_serializer_context(self, *args, **kwargs):
context = super().get_serializer_context(*args, **kwargs)
context.update({
'exclude_utm': get_query_param(self.request, 'exclude_utm'),
'include_deleted_programs': get_query_param(self.request, 'include_deleted_programs'),
})

return context
Expand All @@ -262,6 +263,12 @@ def list(self, request, *args, **kwargs):
type: integer
paramType: query
multiple: false
- name: include_deleted_programs
description: Will include deleted programs in the associated programs array
required: false
type: integer
paramType: query
multiple: false
"""
return super(CourseViewSet, self).list(request, *args, **kwargs)

Expand Down Expand Up @@ -312,6 +319,7 @@ def get_serializer_context(self, *args, **kwargs):
context = super().get_serializer_context(*args, **kwargs)
context.update({
'exclude_utm': get_query_param(self.request, 'exclude_utm'),
'include_deleted_programs': get_query_param(self.request, 'include_deleted_programs'),
})

return context
Expand Down Expand Up @@ -358,6 +366,12 @@ def list(self, request, *args, **kwargs):
type: integer
paramType: query
multiple: false
- name: include_deleted_programs
description: Will include deleted programs in the associated programs array
required: false
type: integer
paramType: query
multiple: false
"""
return super(CourseRunViewSet, self).list(request, *args, **kwargs)

Expand Down

0 comments on commit 910299d

Please sign in to comment.