Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: exclude retired coursetypes and runtypes from api #4525

Merged
merged 19 commits into from
Jan 6, 2025
Merged
5 changes: 4 additions & 1 deletion course_discovery/apps/api/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import pytz
import waffle # lint-amnesty, pylint: disable=invalid-django-waffle-import
from django.conf import settings
from django.contrib.auth import get_user_model
from django.db.models import Count
from django.db.models.query import Prefetch
Expand Down Expand Up @@ -2742,7 +2743,9 @@ def create_type_options(self, info):
],
} for course_type in CourseType.objects.prefetch_related(
'course_run_types__tracks__mode', 'entitlement_types', 'white_listed_orgs'
).exclude(slug=CourseType.EMPTY) if not course_type.white_listed_orgs.exists() or user.is_staff or
).exclude(
slug__in=[CourseType.EMPTY, *settings.RETIRED_COURSE_TYPES]
) if not course_type.white_listed_orgs.exists() or user.is_staff or
user.groups.filter(organization_extension__organization__in=course_type.white_listed_orgs.all()).exists()]
return info

Expand Down
38 changes: 38 additions & 0 deletions course_discovery/apps/api/v1/tests/test_views/test_course_runs.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import contextlib
import datetime
import urllib
import uuid
Expand Down Expand Up @@ -103,6 +104,25 @@ def test_get(self):
assert response.status_code == 200
assert response.data == self.serialize_course_run(self.course_run)

@ddt.data(
[True, 200],
[False, 404]
)
@ddt.unpack
def test_get_filters_retired(self, include_retired, status_code):
""" Verify the endpoint excludes retired run types by default. """
bootcamp_type, _ = CourseRunType.objects.get_or_create(slug=CourseRunType.PAID_BOOTCAMP)
run = CourseRunFactory(course__partner=self.partner, type=bootcamp_type)
url = reverse('api:v1:course_run-detail', kwargs={'key': run.key})
if include_retired:
url += '?include_retired_run_types=1'

context = self.assertNumQueries(15, threshold=3) if include_retired else contextlib.nullcontext(0)
with context:
response = self.client.get(url)

assert response.status_code == status_code

def test_get_exclude_deleted_programs(self):
""" Verify the endpoint returns no associated deleted programs """
ProgramFactory(courses=[self.course_run.course], status=ProgramStatus.Deleted)
Expand Down Expand Up @@ -1208,6 +1228,24 @@ def test_list(self):
self.serialize_course_run(CourseRun.objects.all().order_by(Lower('key')), many=True)
)

@ddt.data(
[True, 3],
[False, 2]
)
@ddt.unpack
def test_list_filters_retired(self, include_retired, expected_length):
""" Verify the endpoint excludes retired types by default. """
bootcamp_type, _ = CourseRunType.objects.get_or_create(slug=CourseRunType.PAID_BOOTCAMP)
CourseRunFactory(course__partner=self.partner, type=bootcamp_type)
url = reverse('api:v1:course_run-list')
if include_retired:
url += '?include_retired_run_types=1'

response = self.client.get(url)

assert response.status_code == 200
assert len(response.data['results']) == expected_length

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
55 changes: 50 additions & 5 deletions course_discovery/apps/api/v1/tests/test_views/test_courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,27 @@ def test_get(self):
assert response.status_code == 200
assert response.data == self.serialize_course(self.course)

@ddt.data(
[True, 200],
[False, 404]
)
@ddt.unpack
def test_get_filters_retired(self, include_retired, status_code):
""" Verify that retired courses types do not appear by default """
bootcamp_type, _ = CourseType.objects.get_or_create(slug=CourseType.BOOTCAMP_2U)
bootcamp = CourseFactory(partner=self.partner, title='Fake Test', key='edX+bootcamp', type=bootcamp_type)
url = reverse('api:v1:course-detail', kwargs={'key': bootcamp.key})
if include_retired:
url += '?include_retired_course_types=1'

response = self.client.get(url)
assert response.status_code == status_code

def test_get_uuid(self):
""" Verify the endpoint returns the details for a single course with UUID. """
url = reverse('api:v1:course-detail', kwargs={'key': self.course.uuid})

with self.assertNumQueries(27):
with self.assertNumQueries(28):
response = self.client.get(url)
assert response.status_code == 200
assert response.data == self.serialize_course(self.course)
Expand Down Expand Up @@ -354,6 +370,24 @@ def test_list(self):
self.serialize_course(Course.objects.all(), many=True)
)

@ddt.data(
[True, 2],
[False, 1]
)
@ddt.unpack
def test_list_filters_retired(self, include_retired, expected_length):
""" Verify that retired course types do not appear by default """
bootcamp_type, _ = CourseType.objects.get_or_create(slug=CourseType.BOOTCAMP_2U)
CourseFactory(partner=self.partner, title='Fake Test', key='edX+bootcamp', type=bootcamp_type)

url = reverse('api:v1:course-list')
if include_retired:
url += '?include_retired_course_types=1'

response = self.client.get(url)
assert response.status_code == 200
assert len(response.data['results']) == expected_length

def test_no_repeated_cache_calls_for_utm_calculation(self):
"""
Test that utm source calculation is done only once per request, and not per
Expand Down Expand Up @@ -507,7 +541,8 @@ def test_list_courses_course_type_filter(self, course_type, expected_length):
CourseFactory(partner=self.partner, title='Fake Test', key='edX+bootcamp', type=bootcamp_type)
CourseFactory(partner=self.partner, title='Fake Test', key='edX+ver', type=self.verified_type)

url = reverse('api:v1:course-list') + '?editable=1&course_type={}'.format(course_type)
query_params = '?editable=1&include_retired_course_types=1&course_type={}'.format(course_type)
url = reverse('api:v1:course-list') + query_params

response = self.client.get(url)
assert response.status_code == 200
Expand Down Expand Up @@ -2589,13 +2624,20 @@ def test_update_geolocation__validation_error(self):
assert GeoLocation.objects.count() == 1

@responses.activate
def test_options(self):
@ddt.data(
[['audit'], False],
[[], True]
)
@ddt.unpack
def test_options(self, retired_course_types, is_retired_type_in_result):
SubjectFactory(name='Subject1')
CourseEntitlementFactory(course=self.course, mode=SeatTypeFactory.verified())
audit_type, _ = CourseType.objects.get_or_create(slug=CourseType.AUDIT)

url = reverse('api:v1:course-detail', kwargs={'key': self.course.uuid})
with self.assertNumQueries(46, threshold=0):
response = self.client.options(url)
with override_settings(RETIRED_COURSE_TYPES=retired_course_types):
with self.assertNumQueries(46, threshold=1):
response = self.client.options(url)
assert response.status_code == 200

data = response.json()['actions']['PUT']
Expand All @@ -2620,6 +2662,9 @@ def test_options(self):
if options['uuid'] == str(credit_type.uuid):
credit_options = options
break
# Assert that retired course types do not appear in the result
type_uuids = [typ['uuid'] for typ in data['type']['type_options']]
assert (str(audit_type.uuid) in type_uuids) == is_retired_type_in_result
assert credit_options is not None
assert {t['mode']['slug'] for t in credit_options['tracks']} == {'verified', 'credit', 'audit'}

Expand Down
8 changes: 7 additions & 1 deletion course_discovery/apps/api/v1/views/course_runs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging

from django.conf import settings
from django.db import models, transaction
from django.db.models.functions import Lower
from django.http.response import Http404
Expand Down Expand Up @@ -27,7 +28,7 @@
from course_discovery.apps.course_metadata.choices import CourseRunStatus
from course_discovery.apps.course_metadata.constants import COURSE_RUN_ID_REGEX
from course_discovery.apps.course_metadata.exceptions import EcommerceSiteAPIClientException
from course_discovery.apps.course_metadata.models import Course, CourseEditor, CourseRun
from course_discovery.apps.course_metadata.models import Course, CourseEditor, CourseRun, CourseRunType
from course_discovery.apps.course_metadata.utils import ensure_draft_world
from course_discovery.apps.publisher.utils import is_publisher_user

Expand Down Expand Up @@ -110,6 +111,11 @@ def get_queryset(self):
)
else:
queryset = queryset.filter(course__partner=partner)
if self.request.method == "GET" and not get_query_param(self.request, 'include_retired_run_types'):
retired_type_ids = list(
CourseRunType.objects.filter(slug__in=settings.RETIRED_RUN_TYPES).values_list('id', flat=True)
)
queryset = queryset.exclude(type_id__in=retired_type_ids)

return self.get_serializer_class().prefetch_queryset(queryset=queryset)

Expand Down
5 changes: 5 additions & 0 deletions course_discovery/apps/api/v1/views/courses.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ def get_queryset(self):
partner=partner,
programs=programs,
)
if self.request.method == 'GET' and not get_query_param(self.request, 'include_retired_course_types'):
retired_type_ids = list(
CourseType.objects.filter(slug__in=settings.RETIRED_COURSE_TYPES).values_list('id', flat=True)
)
queryset = queryset.exclude(type_id__in=retired_type_ids)
if pub_q and edit_mode:
return queryset.filter(Q(key__icontains=pub_q) | Q(title__icontains=pub_q)).order_by('key')

Expand Down
3 changes: 3 additions & 0 deletions course_discovery/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,3 +796,6 @@
# If the keyword is found, the user has more lenient throttling limits.
ENHANCED_THROTTLE_JWT_ROLE_KEYWORDS = []
ENHANCED_THROTTLE_LIMIT = '400/hour'

RETIRED_RUN_TYPES = []
RETIRED_COURSE_TYPES = []
3 changes: 3 additions & 0 deletions course_discovery/settings/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,3 +163,6 @@
ELASTICSEARCH_DSL_LOAD_PER_QUERY = 10000

ENHANCED_THROTTLE_JWT_ROLE_KEYWORDS = ['enterprise']

RETIRED_RUN_TYPES = ['paid-bootcamp', 'unpaid-bootcamp']
RETIRED_COURSE_TYPES = ['bootcamp-2u']
Loading