Skip to content

Commit

Permalink
feat: exclude retired coursetypes and runtypes from api (#4525)
Browse files Browse the repository at this point in the history
  • Loading branch information
zawan-ila authored Jan 6, 2025
1 parent d63d119 commit 88b3ebf
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 7 deletions.
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']

0 comments on commit 88b3ebf

Please sign in to comment.