Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 62 additions & 1 deletion courses/serializers/v1/programs_test.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from datetime import timedelta
from decimal import Decimal
from unittest.mock import ANY

import pytest
from django.utils.timezone import now
Expand Down Expand Up @@ -108,7 +109,7 @@ def test_serialize_program(mock_context, remove_tree, program_with_empty_require
)


def test_program_requirement_tree_serializer_valid():
def test_program_requirement_tree_serializer_save():
"""Verify that the ProgramRequirementTreeSerializer validates data"""
program = ProgramFactory.create()
course1, course2, course3 = CourseFactory.create_batch(3)
Expand Down Expand Up @@ -142,6 +143,66 @@ def test_program_requirement_tree_serializer_valid():
serializer.is_valid(raise_exception=True)
serializer.save()

root.refresh_from_db()
assert ProgramRequirementTreeSerializer(instance=root).data == [
{
"data": {
"node_type": "program_root",
"operator": None,
"operator_value": None,
"program": program.id,
"course": None,
"required_program": None,
"title": "",
"elective_flag": False,
},
"id": ANY,
"children": [
{
"data": {
"node_type": "operator",
"operator": "all_of",
"operator_value": None,
"program": program.id,
"course": None,
"required_program": None,
"title": "Required Courses",
"elective_flag": False,
},
"id": ANY,
"children": [
{
"data": {
"node_type": "course",
"operator": None,
"operator_value": None,
"program": program.id,
"course": course1.id,
"required_program": None,
"title": None,
"elective_flag": False,
},
"id": ANY,
}
],
},
{
"data": {
"node_type": "operator",
"operator": "min_number_of",
"operator_value": "1",
"program": program.id,
"course": None,
"required_program": None,
"title": "Elective Courses",
"elective_flag": False,
},
"id": ANY,
},
],
}
]


def test_program_requirement_deletion():
"""Verify that saving the requirements for one program doesn't affect other programs"""
Expand Down
20 changes: 17 additions & 3 deletions courses/serializers/v2/programs.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,18 @@ class ProgramRequirementDataSerializer(StrictFieldsSerializer):

node_type = serializers.ChoiceField(
choices=(
ProgramRequirementNodeType.OPERATOR,
ProgramRequirementNodeType.COURSE,
ProgramRequirementNodeType.PROGRAM,
ProgramRequirementNodeType.OPERATOR,
)
)
course = serializers.CharField(source="course_id", allow_null=True, default=None)
program = serializers.CharField(source="program_id", required=False)
Comment on lines -31 to -32
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API currently returns integers for this.

I believe we are only using this serializer in read-only viewsets (only v2 ProgramViewSet?), and iirc DRF does not run validation on response data, only request data, which is why it hasn't caused runtime problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Switching to IntegerField fixes the openapi spec, too)

course = serializers.IntegerField(source="course_id", allow_null=True, default=None)
program = serializers.IntegerField(
source="program_id", allow_null=True, default=None
)
required_program = serializers.IntegerField(
source="required_program_id", allow_null=True, default=None
)
Comment on lines +36 to +38
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

required_program is not new—it was added in #2772 and can be seen in the current response at https://mitxonline.mit.edu/api/v2/programs/1

In #2772 it was added to the v1 serializer but not the v2 serializer

title = serializers.CharField(allow_null=True, default=None)
operator = serializers.CharField(allow_null=True, default=None)
operator_value = serializers.CharField(allow_null=True, default=None)
Expand Down Expand Up @@ -79,6 +85,14 @@ class Meta:
class ProgramRequirementTreeSerializer(BaseProgramRequirementTreeSerializer):
child = ProgramRequirementSerializer()

@property
def data(self):
"""Return children of root node directly, or empty array if no children"""
# BaseProgramRequirementTreeSerializer overrides the data property
# to bypass to_implementation, so we do also.
full_data = super().data
return full_data[0].get("children", []) if full_data else []


@extend_schema_serializer(
component_name="V2ProgramSerializer",
Expand Down
113 changes: 113 additions & 0 deletions courses/serializers/v2/programs_test.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
from datetime import timedelta
from unittest.mock import ANY

import pytest
from django.utils.timezone import now

from cms.factories import CoursePageFactory
from cms.serializers import ProgramPageSerializer
from courses.factories import ( # noqa: F401
CourseFactory,
CourseRunFactory,
ProgramCollectionFactory,
ProgramFactory,
Expand Down Expand Up @@ -144,3 +146,114 @@ def test_serialize_program(
"max_price": program_with_empty_requirements.page.max_price,
},
)


def test_program_requirement_tree_serializer_save():
"""Verify that the ProgramRequirementTreeSerializer validates data"""
program = ProgramFactory.create()
course1, course2, course3 = CourseFactory.create_batch(3)
root = program.requirements_root

serializer = ProgramRequirementTreeSerializer(
instance=root,
data=[
{
"data": {
"node_type": "operator",
"title": "Required Courses",
"operator": "all_of",
},
"children": [
{"id": None, "data": {"node_type": "course", "course": course1.id}}
],
},
{
"data": {
"node_type": "operator",
"title": "Elective Courses",
"operator": "min_number_of",
"operator_value": "1",
},
"children": [
{"id": None, "data": {"node_type": "course", "course": course2.id}},
{"id": None, "data": {"node_type": "course", "course": course3.id}},
],
},
],
context={"program": program},
)
serializer.is_valid(raise_exception=True)
serializer.save()

root.refresh_from_db()
assert ProgramRequirementTreeSerializer(instance=root).data == [
{
"data": {
"node_type": "operator",
"operator": "all_of",
"operator_value": None,
"program": program.id,
"course": None,
"required_program": None,
"title": "Required Courses",
"elective_flag": False,
},
"id": ANY,
"children": [
{
"data": {
"node_type": "course",
"operator": None,
"operator_value": None,
"program": program.id,
"course": course1.id,
"required_program": None,
"title": None,
"elective_flag": False,
},
"id": ANY,
}
],
},
{
"data": {
"node_type": "operator",
"operator": "min_number_of",
"operator_value": "1",
"program": program.id,
"course": None,
"required_program": None,
"title": "Elective Courses",
"elective_flag": False,
},
"id": ANY,
"children": [
{
"data": {
"node_type": "course",
"operator": None,
"operator_value": None,
"program": program.id,
"course": course2.id,
"required_program": None,
"title": None,
"elective_flag": False,
},
"id": ANY,
},
{
"data": {
"node_type": "course",
"operator": None,
"operator_value": None,
"program": program.id,
"course": course3.id,
"required_program": None,
"title": None,
"elective_flag": False,
},
"id": ANY,
},
],
},
]
17 changes: 12 additions & 5 deletions openapi/specs/v0.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4884,10 +4884,14 @@ components:
node_type:
$ref: '#/components/schemas/V2ProgramRequirementDataNodeTypeEnum'
course:
type: string
type: integer
nullable: true
program:
type: string
type: integer
nullable: true
required_program:
type: integer
nullable: true
title:
type: string
nullable: true
Expand All @@ -4905,15 +4909,18 @@ components:
- node_type
V2ProgramRequirementDataNodeTypeEnum:
enum:
- operator
- course
- program
- operator
type: string
description: |-
* `operator` - operator
* `course` - course
* `program` - program
* `operator` - operator
x-enum-descriptions:
- operator
- course
- program
- operator
YearsExperienceEnum:
enum:
- 2
Expand Down
17 changes: 12 additions & 5 deletions openapi/specs/v1.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4884,10 +4884,14 @@ components:
node_type:
$ref: '#/components/schemas/V2ProgramRequirementDataNodeTypeEnum'
course:
type: string
type: integer
nullable: true
program:
type: string
type: integer
nullable: true
required_program:
type: integer
nullable: true
title:
type: string
nullable: true
Expand All @@ -4905,15 +4909,18 @@ components:
- node_type
V2ProgramRequirementDataNodeTypeEnum:
enum:
- operator
- course
- program
- operator
type: string
description: |-
* `operator` - operator
* `course` - course
* `program` - program
* `operator` - operator
x-enum-descriptions:
- operator
- course
- program
- operator
YearsExperienceEnum:
enum:
- 2
Expand Down
17 changes: 12 additions & 5 deletions openapi/specs/v2.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4884,10 +4884,14 @@ components:
node_type:
$ref: '#/components/schemas/V2ProgramRequirementDataNodeTypeEnum'
course:
type: string
type: integer
nullable: true
program:
type: string
type: integer
nullable: true
required_program:
type: integer
nullable: true
title:
type: string
nullable: true
Expand All @@ -4905,15 +4909,18 @@ components:
- node_type
V2ProgramRequirementDataNodeTypeEnum:
enum:
- operator
- course
- program
- operator
type: string
description: |-
* `operator` - operator
* `course` - course
* `program` - program
* `operator` - operator
x-enum-descriptions:
- operator
- course
- program
- operator
YearsExperienceEnum:
enum:
- 2
Expand Down