From eb4070f5ce9cc4525bb05444aa48012f1a103d18 Mon Sep 17 00:00:00 2001 From: Simon Chen Date: Wed, 12 Oct 2016 13:56:09 -0400 Subject: [PATCH] Exclude deleted programs from course and course_run api Originally, the associated programs array in the course and course_run api object would include all programs regardless of status. With this change, the deleted programs are filtered. Only the query string parameter include_deleted_programs will list out all associated programs ECOM-5729 --- course_discovery/apps/api/serializers.py | 17 +++- .../apps/api/tests/test_serializers.py | 86 ++++++++++++++++--- .../v1/tests/test_views/test_course_runs.py | 32 ++++++- .../api/v1/tests/test_views/test_courses.py | 34 +++++++- course_discovery/apps/api/v1/views.py | 14 +++ 5 files changed, 161 insertions(+), 22 deletions(-) diff --git a/course_discovery/apps/api/serializers.py b/course_discovery/apps/api/serializers.py index 9830070157..7ee6555411 100644 --- a/course_discovery/apps/api/serializers.py +++ b/course_discovery/apps/api/serializers.py @@ -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 @@ -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 @@ -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 diff --git a/course_discovery/apps/api/tests/test_serializers.py b/course_discovery/apps/api/tests/test_serializers.py index f2f9e4f2f2..30062c3133 100644 --- a/course_discovery/apps/api/tests/test_serializers.py +++ b/course_discovery/apps/api/tests/test_serializers.py @@ -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 @@ -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) @@ -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): diff --git a/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py b/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py index a50938e041..4374c88753 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_course_runs.py @@ -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 @@ -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') diff --git a/course_discovery/apps/api/v1/tests/test_views/test_courses.py b/course_discovery/apps/api/v1/tests/test_views/test_courses.py index c4ee6adf64..12797147ee 100644 --- a/course_discovery/apps/api/v1/tests/test_views/test_courses.py +++ b/course_discovery/apps/api/v1/tests/test_views/test_courses.py @@ -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): @@ -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( @@ -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)) diff --git a/course_discovery/apps/api/v1/views.py b/course_discovery/apps/api/v1/views.py index 53086b1bf3..ca7d955ca8 100644 --- a/course_discovery/apps/api/v1/views.py +++ b/course_discovery/apps/api/v1/views.py @@ -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 @@ -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) @@ -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 @@ -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)