Skip to content

Commit 26fa0c5

Browse files
3689: add department filter to the api endpoints for courses and programs (#2118)
* Working hold * Black formatting * Repair views test * Code review comments * lint
1 parent 8a0cf70 commit 26fa0c5

File tree

7 files changed

+114
-71
lines changed

7 files changed

+114
-71
lines changed

courses/serializers/v2/departments.py

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
from django.db.models import Prefetch, Q, Count
1+
from django.db.models import Q
22
from mitol.common.utils import now_in_utc
33
from rest_framework import serializers
44

@@ -13,13 +13,25 @@ class Meta:
1313
fields = ["id", "name"]
1414

1515

16-
class DepartmentWithCountSerializer(DepartmentSerializer):
16+
class DepartmentWithCoursesAndProgramsSerializer(DepartmentSerializer):
1717
"""Department model serializer that includes the number of courses and programs associated with each"""
1818

19-
courses = serializers.SerializerMethodField()
20-
programs = serializers.SerializerMethodField()
19+
course_ids = serializers.SerializerMethodField()
20+
program_ids = serializers.SerializerMethodField()
2121

22-
def get_courses(self, instance):
22+
def get_course_ids(self, instance):
23+
"""
24+
Returns a list of course IDs associated with courses which are live and
25+
have a CMS page that is live. The associated course runs must be live,
26+
have a start date, enrollment start date in the past, and enrollment end
27+
date in the future or not defined.
28+
29+
Args:
30+
instance (courses.models.Department): Department model instance.
31+
32+
Returns:
33+
list: Course IDs associated with the Department.
34+
"""
2335
now = now_in_utc()
2436
related_courses = instance.course_set.filter(live=True, page__live=True)
2537
relevant_courseruns = CourseRun.objects.filter(
@@ -29,21 +41,32 @@ def get_courses(self, instance):
2941
& Q(enrollment_start__lt=now)
3042
& (Q(enrollment_end=None) | Q(enrollment_end__gt=now))
3143
).values_list("id", flat=True)
32-
3344
return (
3445
related_courses.filter(courseruns__id__in=relevant_courseruns)
3546
.distinct()
36-
.count()
47+
.values_list("id", flat=True)
3748
)
3849

39-
def get_programs(self, instance):
50+
def get_program_ids(self, instance):
51+
"""
52+
Returns a list of program IDs associated with the department
53+
if the program is live and has a live CMS page.
54+
55+
Args:
56+
instance (courses.models.Department): Department model instance.
57+
58+
Returns:
59+
list: Program IDs associated with the Department.
60+
"""
4061
return (
41-
instance.program_set.filter(live=True, page__live=True).distinct().count()
62+
instance.program_set.filter(live=True, page__live=True)
63+
.distinct()
64+
.values_list("id", flat=True)
4265
)
4366

4467
class Meta:
4568
model = Department
4669
fields = DepartmentSerializer.Meta.fields + [
47-
"courses",
48-
"programs",
70+
"course_ids",
71+
"program_ids",
4972
]

courses/serializers/v2/departments_test.py

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
from datetime import timedelta
2-
31
import pytest
42

53
from courses.factories import (
@@ -10,7 +8,7 @@
108
)
119
from courses.serializers.v2.departments import (
1210
DepartmentSerializer,
13-
DepartmentWithCountSerializer,
11+
DepartmentWithCoursesAndProgramsSerializer,
1412
)
1513

1614
from main.test_utils import assert_drf_json_equal
@@ -26,19 +24,33 @@ def test_serialize_department(mock_context):
2624
# Should return 0 when there are no courses or programs at all, or when there are, but none are relevant
2725
def test_serialize_department_with_courses_and_programs__no_related(mock_context):
2826
department = DepartmentFactory.create()
29-
data = DepartmentWithCountSerializer(instance=department, context=mock_context).data
27+
data = DepartmentWithCoursesAndProgramsSerializer(
28+
instance=department, context=mock_context
29+
).data
3030
assert_drf_json_equal(
3131
data,
32-
{"id": department.id, "name": department.name, "courses": 0, "programs": 0},
32+
{
33+
"id": department.id,
34+
"name": department.name,
35+
"course_ids": [],
36+
"program_ids": [],
37+
},
3338
)
3439

3540
course = CourseFactory.create()
3641
CourseRunFactory.create(course=course, in_future=True)
3742
ProgramFactory.create()
38-
data = DepartmentWithCountSerializer(instance=department, context=mock_context).data
43+
data = DepartmentWithCoursesAndProgramsSerializer(
44+
instance=department, context=mock_context
45+
).data
3946
assert_drf_json_equal(
4047
data,
41-
{"id": department.id, "name": department.name, "courses": 0, "programs": 0},
48+
{
49+
"id": department.id,
50+
"name": department.name,
51+
"course_ids": [],
52+
"program_ids": [],
53+
},
4254
)
4355

4456

@@ -54,8 +66,8 @@ def test_serialize_department_with_courses_and_programs__with_multiples(
5466
invalid_programs,
5567
):
5668
department = DepartmentFactory.create()
57-
valid_courses_list = []
58-
valid_programs_list = []
69+
valid_course_id_list = []
70+
valid_program_id_list = []
5971

6072
invalid_courses_list = []
6173
invalid_programs_list = []
@@ -66,25 +78,27 @@ def test_serialize_department_with_courses_and_programs__with_multiples(
6678
# Each course has 2 course runs that are possible matches to make sure it is not returned twice.
6779
CourseRunFactory.create(course=course, in_future=True)
6880
CourseRunFactory.create(course=course, in_progress=True)
69-
valid_courses_list += [course]
81+
valid_course_id_list.append(course.id)
7082
vc -= 1
7183
vp = valid_programs
7284
while vp > 0:
73-
valid_programs_list += [ProgramFactory.create(departments=[department])]
85+
valid_program_id_list.append(ProgramFactory.create(departments=[department]).id)
7486
vp -= 1
7587
while invalid_courses > 0:
76-
invalid_courses_list += [CourseFactory.create()]
88+
# invalid_courses_list += [CourseFactory.create()]
7789
invalid_courses -= 1
7890
while invalid_programs > 0:
79-
invalid_programs_list += [ProgramFactory.create()]
91+
# invalid_programs_list += [ProgramFactory.create()]
8092
invalid_programs -= 1
81-
data = DepartmentWithCountSerializer(instance=department, context=mock_context).data
93+
data = DepartmentWithCoursesAndProgramsSerializer(
94+
instance=department, context=mock_context
95+
).data
8296
assert_drf_json_equal(
8397
data,
8498
{
8599
"id": department.id,
86100
"name": department.name,
87-
"courses": valid_courses,
88-
"programs": valid_programs,
101+
"course_ids": valid_course_id_list,
102+
"program_ids": valid_program_id_list,
89103
},
90104
)

courses/views/v2/__init__.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Course API Views version 2
33
"""
4+
45
from rest_framework import viewsets
56
from rest_framework.pagination import PageNumberPagination
67
import django_filters
@@ -18,7 +19,9 @@
1819
from courses.serializers.v2.courses import (
1920
CourseWithCourseRunsSerializer,
2021
)
21-
from courses.serializers.v2.departments import DepartmentWithCountSerializer
22+
from courses.serializers.v2.departments import (
23+
DepartmentWithCoursesAndProgramsSerializer,
24+
)
2225

2326

2427
class Pagination(PageNumberPagination):
@@ -130,7 +133,7 @@ def get_serializer_context(self):
130133
class DepartmentViewSet(viewsets.ReadOnlyModelViewSet):
131134
"""API view set for Departments"""
132135

133-
serializer_class = DepartmentWithCountSerializer
136+
serializer_class = DepartmentWithCoursesAndProgramsSerializer
134137
pagination_class = Pagination
135138
permission_classes = []
136139

courses/views/v2/views_test.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Tests for courses api views v2
33
"""
4+
45
import logging
56
import random
67

@@ -12,7 +13,9 @@
1213
from courses.factories import DepartmentFactory
1314
from courses.models import Course, Program
1415
from courses.serializers.v2.courses import CourseWithCourseRunsSerializer
15-
from courses.serializers.v2.departments import DepartmentWithCountSerializer
16+
from courses.serializers.v2.departments import (
17+
DepartmentWithCoursesAndProgramsSerializer,
18+
)
1619
from courses.serializers.v2.programs import ProgramSerializer
1720
from courses.views.test_utils import (
1821
num_queries_from_course,
@@ -37,7 +40,7 @@ def test_get_departments(
3740
empty_departments_from_fixture = []
3841
for department in departments:
3942
empty_departments_from_fixture.append(
40-
DepartmentWithCountSerializer(
43+
DepartmentWithCoursesAndProgramsSerializer(
4144
instance=department, context=mock_context
4245
).data
4346
)
@@ -65,7 +68,7 @@ def test_get_departments(
6568
departments_from_fixture = []
6669
for department in departments:
6770
departments_from_fixture.append(
68-
DepartmentWithCountSerializer(
71+
DepartmentWithCoursesAndProgramsSerializer(
6972
instance=department, context=mock_context
7073
).data
7174
)

frontend/public/src/containers/pages/CatalogPage.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,7 @@ export class CatalogPage extends React.Component<Props> {
224224
...new Set([
225225
ALL_DEPARTMENTS,
226226
...departments.flatMap(department =>
227-
department.courses > 0 ? department.name : []
227+
department.course_ids.length > 0 ? department.name : []
228228
)
229229
])
230230
]
@@ -233,7 +233,7 @@ export class CatalogPage extends React.Component<Props> {
233233
...new Set([
234234
ALL_DEPARTMENTS,
235235
...departments.flatMap(department =>
236-
department.programs > 0 ? department.name : []
236+
department.program_ids.length > 0 ? department.name : []
237237
)
238238
])
239239
]
@@ -311,9 +311,10 @@ export class CatalogPage extends React.Component<Props> {
311311
}
312312

313313
/**
314-
* Returns a filtered array of Course Runs which are live, define a start date,
315-
* enrollment start date is before the current date and time, and
316-
* enrollment end date is not defined or is after the current date and time.
314+
* Returns a filtered array of Course Runs which are live and:
315+
* - Have a start_date before the current date and time
316+
* - Have an enrollment_start_date that is before the current date and time
317+
* - Has an enrollment_end_date that is not defined or is after the current date and time.
317318
* @param {Array<BaseCourseRun>} courseRuns The array of Course Runs apply the filter to.
318319
*/
319320
validateCoursesCourseRuns(courseRuns: Array<BaseCourseRun>) {
@@ -379,7 +380,7 @@ export class CatalogPage extends React.Component<Props> {
379380
} else if (this.state.selectedDepartment !== ALL_DEPARTMENTS) {
380381
return departments.find(
381382
department => department.name === this.state.selectedDepartment
382-
).courses
383+
).course_ids.length
383384
}
384385
}
385386

@@ -390,7 +391,7 @@ export class CatalogPage extends React.Component<Props> {
390391
} else if (this.state.selectedDepartment !== ALL_DEPARTMENTS) {
391392
return departments.find(
392393
department => department.name === this.state.selectedDepartment
393-
).programs
394+
).program_ids.length
394395
} else return 0
395396
}
396397

frontend/public/src/containers/pages/CatalogPage_test.js

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -223,14 +223,14 @@ describe("CatalogPage", function() {
223223
},
224224
departments: [
225225
{
226-
name: "History",
227-
courses: 0,
228-
programs: 5
226+
name: "History",
227+
course_ids: [],
228+
program_ids: [1, 2, 3, 4, 5]
229229
},
230230
{
231-
name: "Science",
232-
courses: 1,
233-
programs: 0
231+
name: "Science",
232+
course_ids: [2],
233+
program_ids: []
234234
}
235235
]
236236
}
@@ -262,24 +262,24 @@ describe("CatalogPage", function() {
262262
entities: {
263263
departments: [
264264
{
265-
name: "department1",
266-
courses: 1,
267-
programs: 0
265+
name: "department1",
266+
course_ids: [1],
267+
program_ids: []
268268
},
269269
{
270-
name: "department2",
271-
courses: 1,
272-
programs: 1
270+
name: "department2",
271+
course_ids: [1],
272+
program_ids: [1]
273273
},
274274
{
275-
name: "department3",
276-
courses: 0,
277-
programs: 1
275+
name: "department3",
276+
course_ids: [],
277+
program_ids: [1]
278278
},
279279
{
280-
name: "department4",
281-
courses: 0,
282-
programs: 0
280+
name: "department4",
281+
course_ids: [],
282+
program_ids: []
283283
}
284284
]
285285
}
@@ -522,19 +522,19 @@ describe("CatalogPage", function() {
522522
},
523523
departments: [
524524
{
525-
name: "History",
526-
courses: 2,
527-
programs: 1
525+
name: "History",
526+
course_ids: [1, 2],
527+
program_ids: [1]
528528
},
529529
{
530-
name: "Math",
531-
courses: 3,
532-
programs: 0
530+
name: "Math",
531+
course_ids: [1, 2, 3],
532+
program_ids: []
533533
},
534534
{
535-
name: "department4",
536-
courses: 0,
537-
programs: 0
535+
name: "department4",
536+
course_ids: [],
537+
program_ids: []
538538
}
539539
]
540540
}
@@ -799,9 +799,9 @@ describe("CatalogPage", function() {
799799
},
800800
departments: [
801801
{
802-
name: "History",
803-
courses: 1,
804-
programs: 1
802+
name: "History",
803+
course_ids: [1],
804+
program_ids: [1]
805805
}
806806
]
807807
}

0 commit comments

Comments
 (0)