Skip to content

Commit

Permalink
Cleaned up unused code to move closer to open edX codebase EDLY-2704 (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
Muhammad Haseeb authored Jun 5, 2021
1 parent 9932698 commit 7bf1cb4
Show file tree
Hide file tree
Showing 11 changed files with 13 additions and 252 deletions.
23 changes: 7 additions & 16 deletions course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from course_discovery.apps.api.fields import (
HtmlField, ImageField, SlugRelatedFieldWithReadSerializer, SlugRelatedTranslatableField, StdImageSerializerField
)
from course_discovery.apps.api.utils import StudioAPI, get_queryset_filtered_on_organization
from course_discovery.apps.api.utils import StudioAPI
from course_discovery.apps.catalogs.models import Catalog
from course_discovery.apps.core.api_client.lms import LMSAPIClient
from course_discovery.apps.course_metadata import search_indexes
Expand Down Expand Up @@ -779,7 +779,7 @@ class Meta:
model = CourseRun
fields = ('key', 'uuid', 'title', 'external_key', 'image', 'short_description', 'marketing_url',
'seats', 'start', 'end', 'go_live_date', 'enrollment_start', 'enrollment_end',
'pacing_type', 'type', 'run_type', 'status', 'is_enrollable', 'is_marketable', 'term',)
'pacing_type', 'type', 'run_type', 'status', 'is_enrollable', 'is_marketable', 'term', 'subjects',)

def get_marketing_url(self, obj):
include_archived = self.context.get('include_archived')
Expand Down Expand Up @@ -944,10 +944,8 @@ class CourseRunWithProgramsSerializer(CourseRunSerializer):
programs = serializers.SerializerMethodField()

@classmethod
def prefetch_queryset(cls, queryset=None, edx_org_short_name=None):
edx_org_filter = 'course__authoring_organizations__key'
def prefetch_queryset(cls, queryset=None):
queryset = super().prefetch_queryset(queryset=queryset)
queryset = get_queryset_filtered_on_organization(queryset, edx_org_filter, edx_org_short_name)

return queryset.prefetch_related('course__programs__excluded_course_runs')

Expand Down Expand Up @@ -1128,15 +1126,12 @@ class CourseWithProgramsSerializer(CourseSerializer):
editable = serializers.SerializerMethodField()

@classmethod
def prefetch_queryset(cls, partner, edx_org_short_name=None, queryset=None, course_runs=None, programs=None):
def prefetch_queryset(cls, partner, queryset=None, course_runs=None, programs=None): # pylint: disable=arguments-differ
"""
Similar to the CourseSerializer's prefetch_queryset, but prefetches a
filtered CourseRun queryset.
"""
edx_org_filter = 'authoring_organizations__key'
queryset = queryset if queryset is not None else Course.objects.filter(partner=partner)

queryset = get_queryset_filtered_on_organization(queryset, edx_org_filter, edx_org_short_name)
return queryset.select_related(
'level_type',
'video',
Expand All @@ -1150,7 +1145,7 @@ def prefetch_queryset(cls, partner, edx_org_short_name=None, queryset=None, cour
'prerequisites',
'topics',
'url_slug_history',
Prefetch('subjects', queryset=SubjectSerializer.prefetch_queryset()),
Prefetch('subjects', queryset=SubjectSerializer.prefetch_queryset(partner)),
Prefetch('course_runs', queryset=CourseRunSerializer.prefetch_queryset(queryset=course_runs)),
Prefetch('authoring_organizations', queryset=OrganizationSerializer.prefetch_queryset(partner)),
Prefetch('sponsoring_organizations', queryset=OrganizationSerializer.prefetch_queryset(partner)),
Expand Down Expand Up @@ -1436,11 +1431,9 @@ class MinimalProgramSerializer(DynamicFieldsMixin, BaseModelSerializer):
curricula = CurriculumSerializer(many=True)

@classmethod
def prefetch_queryset(cls, partner, queryset=None, edx_org_short_name=None):
def prefetch_queryset(cls, partner, queryset=None):
# Explicitly check if the queryset is None before selecting related
edx_org_filter = 'authoring_organizations__key'
queryset = queryset if queryset is not None else Program.objects.filter(partner=partner)
queryset = get_queryset_filtered_on_organization(queryset, edx_org_filter, edx_org_short_name)

return queryset.select_related('type', 'partner').prefetch_related(
'excluded_course_runs',
Expand Down Expand Up @@ -1577,17 +1570,15 @@ class ProgramSerializer(MinimalProgramSerializer):
curricula = CurriculumSerializer(many=True, required=False)

@classmethod
def prefetch_queryset(cls, partner, queryset=None, edx_org_short_name=None):
def prefetch_queryset(cls, partner, queryset=None):
"""
Prefetch the related objects that will be serialized with a `Program`.
We use Prefetch objects so that we can prefetch and select all the way down the
chain of related fields from programs to course runs (i.e., we want control over
the querysets that we're prefetching).
"""
edx_org_filter = 'authoring_organizations__key'
queryset = queryset if queryset is not None else Program.objects.filter(partner=partner)
queryset = get_queryset_filtered_on_organization(queryset, edx_org_filter, edx_org_short_name)

return queryset.select_related('type', 'video', 'partner').prefetch_related(
'excluded_course_runs',
Expand Down
63 changes: 0 additions & 63 deletions course_discovery/apps/api/tests/test_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -468,18 +468,6 @@ def test_advertise_course_run_else_condition(self):
serializer = self.serializer_class(self.course, context={'request': self.request})
self.assertEqual(serializer.data['advertised_course_run_uuid'], expected_advertised_course_run.uuid)

def test_filter_courses_on_edx_org_short_name(self):
"""
Verify courses filtering on edX organization works as expected..
"""
edx_org_short_name = 'edx'
edx_org_course = CourseFactory(partner=self.partner, authoring_organizations__key=edx_org_short_name)
serialized_courses = self.serializer_class.prefetch_queryset(
partner=self.partner,
edx_org_short_name=edx_org_short_name
)
assert serialized_courses.values() != self.get_expected_data(edx_org_course, self.request).values()


class CurriculumSerializerTests(TestCase):
serializer_class = CurriculumSerializer
Expand Down Expand Up @@ -743,17 +731,6 @@ def test_include_retired_programs(self):
NestedProgramSerializer([retired_program], many=True, context=self.serializer_context).data
)

def test_filter_course_runs_on_edx_org_short_name(self):
"""
Verify course runs filtering on edX organization works as expected.
"""
edx_org_short_name = 'edx'
edx_org_course = CourseRunFactory(course__authoring_organizations__key=edx_org_short_name)
serialized_course_runs = CourseRunWithProgramsSerializer.prefetch_queryset(
edx_org_short_name=edx_org_short_name
)
assert serialized_course_runs.values() != self.get_expected_data(edx_org_course, self.request).values()

@classmethod
def get_expected_data(cls, course_run, request):
expected = CourseRunSerializer(course_run, context={'request': request}).data
Expand Down Expand Up @@ -1039,16 +1016,6 @@ def test_data(self):
expected = self.get_expected_data(program, request)
self.assertDictEqual(serializer.data, expected)

def test_filter_programs_on_edx_org_short_name(self):
"""
Verify programs filtering on edX organization works as expected.
"""
edx_org_short_name = 'edx'
request = make_request()
edx_org_program = ProgramFactory(authoring_organizations__key=edx_org_short_name)
serializer = self.serializer_class(edx_org_program, context={'request': request})
assert serializer.data['title'] == self.get_expected_data(edx_org_program, request)['title']


class ProgramSerializerTests(MinimalProgramSerializerTests):
serializer_class = ProgramSerializer
Expand Down Expand Up @@ -1345,16 +1312,6 @@ def test_degree_marketing_data(self):
}
self.assertDictEqual(serializer.data, expected)

def test_filter_programs_on_edx_org_short_name(self):
"""
Verify programs filtering on edX organization works as expected.
"""
edx_org_short_name = 'edx'
request = make_request()
edx_org_program = ProgramFactory(authoring_organizations__key=edx_org_short_name)
serializer = self.serializer_class(edx_org_program, context={'request': request})
assert serializer.data['title'] == self.get_expected_data(edx_org_program, request)['title']


class PathwaySerialzerTests(TestCase):
def test_data(self):
Expand Down Expand Up @@ -1402,7 +1359,6 @@ def test_data(self):


class ContainedCourseRunsSerializerTests(TestCase):

def test_data(self):
instance = {
'course_runs': {
Expand All @@ -1413,25 +1369,6 @@ def test_data(self):
serializer = ContainedCourseRunsSerializer(instance)
self.assertDictEqual(serializer.data, instance)

def test_contained_course_runs_filtered_on_edx_org_short_name(self):
"""
Verify course runs keys filtering on elastic search query string & edX organization.
"""
edx_org_short_name = 'edx'
edx_org_course_run_key = 'course-v1:edX+DemoX+Demo_Course'
course_run_ids = [edx_org_course_run_key]
course_runs_with_org_filter = CourseRun.objects.filter(course__authoring_organizations__key=edx_org_short_name)
course_runs_keys = course_runs_with_org_filter.values_list('key', flat=True)
contained_course_runs = {course_run_id: course_run_id in course_runs_keys for course_run_id in course_run_ids}
course_runs_instances = {'course_runs': contained_course_runs}
contained_course_runs_serializer = ContainedCourseRunsSerializer(course_runs_instances)
expected_contained_courses = {
'course_runs': {
edx_org_course_run_key: False
}
}
assert contained_course_runs_serializer.data == expected_contained_courses


class ContainedCoursesSerializerTests(TestCase):
def test_data(self):
Expand Down
31 changes: 1 addition & 30 deletions course_discovery/apps/api/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@

import ddt
import mock
import pytest
from django.test import TestCase
from opaque_keys.edx.keys import CourseKey
from rest_framework.request import Request
from rest_framework.test import APIRequestFactory

from course_discovery.apps.api.utils import StudioAPI, cast2int, get_query_param, get_queryset_filtered_on_organization
from course_discovery.apps.api.utils import StudioAPI, cast2int, get_query_param
from course_discovery.apps.core.utils import serialize_datetime
from course_discovery.apps.course_metadata.models import Course, CourseRun
from course_discovery.apps.course_metadata.tests.factories import CourseEditorFactory, CourseRunFactory

LOGGER_PATH = 'course_discovery.apps.api.utils.logger.exception'
Expand Down Expand Up @@ -50,33 +48,6 @@ def test_without_request(self):
assert get_query_param(None, 'q') is None


class TestGetQuerysetFilteredOnOrganization:

@pytest.mark.django_db
def test_courses_runs_filter_on_edx_shortname(self):
edx_org_filter = 'course__authoring_organizations__key'
edx_org_short_name = 'edx'
edx_course_run_key = 'course-v1:edX+DemoX+Demo_Course'
expected_course_runs_queryset = CourseRun.objects.filter(course__authoring_organizations__key=edx_org_short_name)

course_runs_queryset = CourseRun.objects.filter(key=edx_course_run_key)
actual_course_runs_queryset = get_queryset_filtered_on_organization(course_runs_queryset, edx_org_filter, edx_org_short_name)

assert len(actual_course_runs_queryset) == len(expected_course_runs_queryset)

@pytest.mark.django_db
def test_course_filter_on_edx_shortname(self):
edx_org_filter = 'authoring_organizations__key'
edx_org_short_name = 'edx'
edx_course_key = 'course-v1:edX+DemoX+Demo_Course'
expected_courses_queryset = Course.objects.filter(authoring_organizations__key=edx_org_short_name)

courses_queryset = Course.objects.filter(key=edx_course_key)
actual_courses_queryset = get_queryset_filtered_on_organization(courses_queryset, edx_org_filter, edx_org_short_name)

assert len(actual_courses_queryset) == len(expected_courses_queryset)


@ddt.ddt
class StudioAPITests(TestCase):
def setUp(self):
Expand Down
15 changes: 0 additions & 15 deletions course_discovery/apps/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,18 +208,3 @@ def push_to_studio(self, course_run, create=False, old_course_run_key=None, user
response = self.update_course_run_details_in_studio(course_run)

return response


def get_queryset_filtered_on_organization(queryset, edx_org_filter, edx_org_short_name):
"""
Get queryset filtered on edx organization short name.
Arguments:
queryset (DRF queryset object): DRF Queryset object.
edx_org_filter (str): Filter to use in queryset.
edx_org_short_name (str): Edx organization short name.
Returns:
A DRF queryset object with organization filter applied.
"""
return queryset if not edx_org_short_name else queryset.filter(**{edx_org_filter: edx_org_short_name})
55 changes: 0 additions & 55 deletions course_discovery/apps/api/v1/tests/test_views/test_course_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,9 @@ class CourseRunViewSetTests(SerializationMixin, ElasticsearchTestMixin, OAuth2Mi
def setUp(self):
super(CourseRunViewSetTests, self).setUp()
self.user = UserFactory(is_staff=True)
self.edx_org_short_name = 'edx'
self.client.force_authenticate(self.user)
self.course_run = CourseRunFactory(course__partner=self.partner)
self.course_run_2 = CourseRunFactory(course__key='Test+Course', course__partner=self.partner)
# Course_run of edx organization
self.course_run_3 = CourseRunFactory(
course__partner=self.partner,
course__authoring_organizations__key=self.edx_org_short_name
)
self.draft_course = CourseFactory(partner=self.partner, draft=True)
self.draft_course_run = CourseRunFactory(course=self.draft_course, draft=True)
self.draft_course_run.course.authoring_organizations.add(OrganizationFactory(key='course-id'))
Expand Down Expand Up @@ -996,24 +990,6 @@ def test_list(self):
self.serialize_course_run(CourseRun.objects.all().order_by(Lower('key')), many=True)
)

def test_list_edx_org_short_name_filter(self):
"""
Verify course runs filtering on edX organization.
"""
course_run_api_url_with_org_filter = '{course_run_api_url}?org={edx_org_short_name}'.format(
course_run_api_url=reverse('api:v1:course_run-list'),
edx_org_short_name=self.edx_org_short_name
)
expected_serialized_course_runs = self.serialize_course_run(
CourseRun.objects.filter(course__authoring_organizations__key=self.edx_org_short_name).order_by(Lower('key')),
many=True
)

response = self.client.get(course_run_api_url_with_org_filter)
assert response.status_code == 200
course_runs_from_response = response.data['results']
assert course_runs_from_response == expected_serialized_course_runs

def test_list_sorted_by_course_start_date(self):
""" Verify the endpoint returns a list of all course runs sorted by start date. """
url = '{root}?ordering=start'.format(root=reverse('api:v1:course_run-list'))
Expand Down Expand Up @@ -1158,37 +1134,6 @@ def test_contains_multiple_course_runs(self):
}
)

def test_contains_multiple_course_runs_edx_org_short_name_filter(self):
"""
Verify contained course runs filtering on edX organization.
"""
edx_org_course_run_key = 'course-v1:edX+DemoX+Demo_Course'
elastic_search_query_string_with_org_filter = urllib.parse.urlencode({
'query': 'id:course*',
'course_run_ids': '{course_run_1_key},{course_run_2_key},{course_run_3_key}'.format(
course_run_1_key=self.course_run_2.key,
course_run_2_key=self.course_run_3.key,
course_run_3_key=edx_org_course_run_key
),
'org': self.edx_org_short_name
})
course_run_api_url_with_org_filter = '{course_run_api_url}?{elastic_search_query_string_with_org_filter}'.format(
course_run_api_url=reverse('api:v1:course_run-contains'),
elastic_search_query_string_with_org_filter=elastic_search_query_string_with_org_filter
)
expected_serialized_contained_course_runs = {
'course_runs': {
self.course_run_2.key: False,
self.course_run_3.key: False,
edx_org_course_run_key: False
}
}

response = self.client.get(course_run_api_url_with_org_filter)
assert response.status_code == 200
course_runs_from_response = response.data
assert course_runs_from_response == expected_serialized_contained_course_runs

@ddt.data(
{'params': {'course_run_ids': 'a/b/c'}},
{'params': {'query': 'id:course*'}},
Expand Down
19 changes: 0 additions & 19 deletions course_discovery/apps/api/v1/tests/test_views/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def setUp(self):
self.course = CourseFactory(partner=self.partner, title='Fake Test', key='edX+Fake101', type=self.audit_type)
self.org = OrganizationFactory(key='edX', partner=self.partner)
self.course.authoring_organizations.add(self.org) # pylint: disable=no-member
self.edx_org_short_name = 'edx'

def tearDown(self):
super(CourseViewSetTests, self).tearDown()
Expand Down Expand Up @@ -230,24 +229,6 @@ def test_list(self):
self.serialize_course(Course.objects.all().order_by(Lower('key')), many=True)
)

def test_edx_org_short_name_filter(self):
"""
Verify courses filtering on edX organization.
"""
course_api_url_with_org_filter = '{courses_api_url}?org={edx_org_short_name}'.format(
courses_api_url=reverse('api:v1:course-list'),
edx_org_short_name=self.edx_org_short_name
)
expected_serialized_courses = self.serialize_course(
Course.objects.filter(authoring_organizations__key=self.edx_org_short_name).order_by(Lower('key')),
many=True
)

response = self.client.get(course_api_url_with_org_filter)
assert response.status_code == 200
courses_from_response = response.data['results']
assert courses_from_response == expected_serialized_courses

def test_list_query(self):
""" Verify the endpoint returns a filtered list of courses """
title = 'Some random title'
Expand Down
Loading

0 comments on commit 7bf1cb4

Please sign in to comment.