Skip to content

Commit 79bea46

Browse files
Taimoor  AhmedTaimoor  Ahmed
authored andcommitted
feat: standardize serializer usage in EnrollmentView and EnrollmentAllowedView (ADR 0025)
Migrate EnrollmentView and EnrollmentAllowedView to explicit DRF serializer usage for both input validation and response formatting, per the decision recorded in docs/decisions/0025-standardize-serializer-usage.rst.
1 parent 2e2ebb9 commit 79bea46

3 files changed

Lines changed: 461 additions & 83 deletions

File tree

openedx/core/djangoapps/enrollments/serializers.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,3 +137,22 @@ class Meta:
137137
model = CourseEnrollmentAllowed
138138
exclude = ["id"]
139139
lookup_field = "user"
140+
141+
142+
class UserRoleSerializer(serializers.Serializer): # pylint: disable=abstract-method
143+
"""Serializes a single course-level role entry for a user."""
144+
145+
org = serializers.CharField()
146+
course_id = serializers.SerializerMethodField()
147+
role = serializers.CharField()
148+
149+
def get_course_id(self, obj):
150+
"""Return course_id as a string."""
151+
return str(obj.course_id)
152+
153+
154+
class UserRolesResponseSerializer(serializers.Serializer): # pylint: disable=abstract-method
155+
"""Serializes the full response payload for EnrollmentUserRolesView."""
156+
157+
roles = UserRoleSerializer(many=True)
158+
is_staff = serializers.BooleanField()

openedx/core/djangoapps/enrollments/tests/test_views.py

Lines changed: 348 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
from openedx.core.djangoapps.course_groups import cohorts
3737
from openedx.core.djangoapps.embargo.models import Country, CountryAccessRule, RestrictedCourse
3838
from openedx.core.djangoapps.embargo.test_utils import restrict_course
39-
from openedx.core.djangoapps.enrollments import api, data
39+
from openedx.core.djangoapps.enrollments import data
4040
from openedx.core.djangoapps.enrollments.errors import CourseEnrollmentError
4141
from openedx.core.djangoapps.enrollments.views import EnrollmentUserThrottle
4242
from openedx.core.djangoapps.notifications.config.waffle import ENABLE_NOTIFICATIONS
@@ -711,9 +711,9 @@ def test_get_enrollment_details_bad_course(self):
711711
)
712712
assert resp.status_code == status.HTTP_400_BAD_REQUEST
713713

714-
@patch.object(api, "get_enrollment")
715-
def test_get_enrollment_internal_error(self, mock_get_enrollment):
716-
mock_get_enrollment.side_effect = CourseEnrollmentError("Something bad happened.")
714+
@patch.object(CourseEnrollment.objects, "get")
715+
def test_get_enrollment_internal_error(self, mock_get):
716+
mock_get.side_effect = CourseEnrollmentError("Something bad happened.")
717717
resp = self.client.get(
718718
reverse(
719719
'courseenrollment',
@@ -2031,3 +2031,347 @@ def test_delete_enrollment_allowed(self, delete_data, expected_result):
20312031
self.client.post(self.url, self.data)
20322032
response = self.client.delete(self.url, delete_data)
20332033
assert response.status_code == expected_result
2034+
2035+
# --- Response-shape tests (ADR 0025 serializer migration) ---
2036+
2037+
def test_post_response_shape(self):
2038+
"""POST 201 response contains the expected fields from CourseEnrollmentAllowedSerializer."""
2039+
response = self.client.post(self.url, self.data)
2040+
assert response.status_code == status.HTTP_201_CREATED
2041+
body = response.json()
2042+
assert body['email'] == self.data['email']
2043+
assert body['course_id'] == self.data['course_id']
2044+
assert body['auto_enroll'] is False
2045+
assert 'created' in body
2046+
2047+
def test_post_auto_enroll_true_in_response(self):
2048+
"""POST with auto_enroll=true is reflected in the 201 response."""
2049+
response = self.client.post(self.url, {**self.data, 'auto_enroll': True})
2050+
assert response.status_code == status.HTTP_201_CREATED
2051+
assert response.json()['auto_enroll'] is True
2052+
2053+
def test_post_missing_email_returns_field_error(self):
2054+
"""POST without email returns a serializer field-level 400 with an 'email' key."""
2055+
response = self.client.post(self.url, {'course_id': self.data['course_id']})
2056+
assert response.status_code == status.HTTP_400_BAD_REQUEST
2057+
assert 'email' in response.json()
2058+
2059+
def test_post_missing_course_id_returns_field_error(self):
2060+
"""POST without course_id returns a serializer field-level 400 with a 'course_id' key."""
2061+
response = self.client.post(self.url, {'email': self.data['email']})
2062+
assert response.status_code == status.HTTP_400_BAD_REQUEST
2063+
assert 'course_id' in response.json()
2064+
2065+
def test_post_duplicate_returns_409_with_message(self):
2066+
"""A duplicate POST returns 409 with a 'message' key."""
2067+
self.client.post(self.url, self.data)
2068+
response = self.client.post(self.url, self.data)
2069+
assert response.status_code == status.HTTP_409_CONFLICT
2070+
assert 'message' in response.json()
2071+
2072+
def test_get_response_is_list(self):
2073+
"""GET response body is a JSON list."""
2074+
response = self.client.get(self.url, {'email': self.data['email']})
2075+
assert response.status_code == status.HTTP_200_OK
2076+
assert isinstance(response.json(), list)
2077+
2078+
def test_get_empty_response_is_empty_list(self):
2079+
"""GET with no matching enrollments returns an empty list, not null."""
2080+
response = self.client.get(self.url, {'email': 'nobody@example.com'})
2081+
assert response.status_code == status.HTTP_200_OK
2082+
assert response.json() == []
2083+
2084+
def test_get_item_shape(self):
2085+
"""Each item in the GET response has the fields from CourseEnrollmentAllowedSerializer."""
2086+
self.client.post(self.url, self.data)
2087+
response = self.client.get(self.url, {'email': self.data['email']})
2088+
assert response.status_code == status.HTTP_200_OK
2089+
item = response.json()[0]
2090+
assert item['email'] == self.data['email']
2091+
assert item['course_id'] == self.data['course_id']
2092+
assert 'auto_enroll' in item
2093+
assert 'created' in item
2094+
2095+
def test_get_multiple_entries_returned(self):
2096+
"""GET returns all enrollment-allowed records for a given email."""
2097+
second_course = 'course-v1:edX+OtherX+Other_Course'
2098+
self.client.post(self.url, self.data)
2099+
self.client.post(self.url, {'email': self.data['email'], 'course_id': second_course})
2100+
response = self.client.get(self.url, {'email': self.data['email']})
2101+
assert response.status_code == status.HTTP_200_OK
2102+
results = response.json()
2103+
assert len(results) == 2
2104+
assert all(r['email'] == self.data['email'] for r in results)
2105+
2106+
def test_delete_missing_email_returns_field_error(self):
2107+
"""DELETE without email returns a serializer field-level 400 with an 'email' key."""
2108+
self.client.post(self.url, self.data)
2109+
response = self.client.delete(self.url, {'course_id': self.data['course_id']})
2110+
assert response.status_code == status.HTTP_400_BAD_REQUEST
2111+
assert 'email' in response.json()
2112+
2113+
2114+
@skip_unless_lms
2115+
class EnrollmentViewResponseShapeTest(ModuleStoreTestCase, APITestCase):
2116+
"""
2117+
Tests that verify EnrollmentView (GET /enrollment/v1/enrollment/{course_id} and
2118+
/enrollment/v1/enrollment/{username},{course_id}) response structure is preserved
2119+
after migrating to direct serializer usage (ADR 0025).
2120+
"""
2121+
2122+
USERNAME = "Bob"
2123+
PASSWORD = "edx"
2124+
2125+
def setUp(self):
2126+
super().setUp()
2127+
self.course = CourseFactory.create(emit_signals=True)
2128+
self.user = UserFactory.create(username=self.USERNAME, password=self.PASSWORD)
2129+
self.client.login(username=self.USERNAME, password=self.PASSWORD)
2130+
CourseModeFactory.create(
2131+
course_id=self.course.id,
2132+
mode_slug=CourseMode.DEFAULT_MODE_SLUG,
2133+
mode_display_name=CourseMode.DEFAULT_MODE_SLUG,
2134+
)
2135+
CourseEnrollment.enroll(self.user, self.course.id)
2136+
2137+
def _get_by_course_id(self):
2138+
return self.client.get(
2139+
reverse('courseenrollment', kwargs={'course_id': str(self.course.id)})
2140+
)
2141+
2142+
def _get_by_username_and_course_id(self):
2143+
return self.client.get(
2144+
reverse('courseenrollment', kwargs={'username': self.USERNAME, 'course_id': str(self.course.id)})
2145+
)
2146+
2147+
def test_get_by_course_id_returns_200(self):
2148+
assert self._get_by_course_id().status_code == status.HTTP_200_OK
2149+
2150+
def test_get_by_username_course_id_returns_200(self):
2151+
assert self._get_by_username_and_course_id().status_code == status.HTTP_200_OK
2152+
2153+
def test_get_response_top_level_fields(self):
2154+
"""Response contains the expected top-level enrollment fields."""
2155+
body = self._get_by_course_id().json()
2156+
for field in ('created', 'mode', 'is_active', 'user', 'course_details'):
2157+
assert field in body, f"Missing top-level field: {field}"
2158+
2159+
def test_get_response_user_and_mode(self):
2160+
"""user and mode values match the enrollment."""
2161+
body = self._get_by_course_id().json()
2162+
assert body['user'] == self.USERNAME
2163+
assert body['mode'] == CourseMode.DEFAULT_MODE_SLUG
2164+
assert body['is_active'] is True
2165+
2166+
def test_get_by_username_course_id_matches_by_course_id(self):
2167+
"""Both URL shapes return identical response bodies."""
2168+
by_course = self._get_by_course_id().json()
2169+
by_username = self._get_by_username_and_course_id().json()
2170+
assert by_course == by_username
2171+
2172+
def test_get_course_details_fields(self):
2173+
"""course_details contains the expected nested fields."""
2174+
course_details = self._get_by_course_id().json()['course_details']
2175+
for field in (
2176+
'course_id', 'course_name', 'enrollment_start', 'enrollment_end',
2177+
'course_start', 'course_end', 'invite_only', 'course_modes', 'pacing_type',
2178+
):
2179+
assert field in course_details, f"Missing course_details field: {field}"
2180+
assert course_details['course_id'] == str(self.course.id)
2181+
2182+
def test_get_no_enrollment_returns_null(self):
2183+
"""GET for a course the user never enrolled in returns HTTP 200 with a null body."""
2184+
unenrolled_course = CourseFactory.create(emit_signals=True)
2185+
resp = self.client.get(
2186+
reverse('courseenrollment', kwargs={'course_id': str(unenrolled_course.id)})
2187+
)
2188+
assert resp.status_code == status.HTTP_200_OK
2189+
assert resp.json() is None
2190+
2191+
2192+
@skip_unless_lms
2193+
class EnrollmentCourseDetailViewResponseShapeTest(ModuleStoreTestCase, APITestCase):
2194+
"""
2195+
Tests that verify EnrollmentCourseDetailView (GET /enrollment/v1/course/{course_id})
2196+
response structure is preserved after migrating to CourseSerializer + direct ORM (ADR 0025).
2197+
"""
2198+
2199+
def setUp(self):
2200+
super().setUp()
2201+
self.course = CourseFactory.create(emit_signals=True)
2202+
CourseModeFactory.create(
2203+
course_id=self.course.id,
2204+
mode_slug=CourseMode.DEFAULT_MODE_SLUG,
2205+
mode_display_name=CourseMode.DEFAULT_MODE_SLUG,
2206+
)
2207+
2208+
def _get_course_details(self, course_id=None, include_expired=False):
2209+
url = reverse('courseenrollmentdetails', kwargs={'course_id': course_id or str(self.course.id)})
2210+
if include_expired:
2211+
url += '?include_expired=1'
2212+
return self.client.get(url)
2213+
2214+
def test_returns_200(self):
2215+
assert self._get_course_details().status_code == status.HTTP_200_OK
2216+
2217+
def test_response_top_level_fields(self):
2218+
"""Response contains the expected top-level CourseSerializer fields."""
2219+
body = self._get_course_details().json()
2220+
for field in ('course_id', 'course_name', 'enrollment_start', 'enrollment_end',
2221+
'course_start', 'course_end', 'invite_only', 'course_modes', 'pacing_type'):
2222+
assert field in body, f"Missing field: {field}"
2223+
2224+
def test_course_id_matches_requested_course(self):
2225+
body = self._get_course_details().json()
2226+
assert body['course_id'] == str(self.course.id)
2227+
2228+
def test_course_modes_is_list(self):
2229+
body = self._get_course_details().json()
2230+
assert isinstance(body['course_modes'], list)
2231+
2232+
def test_course_mode_fields(self):
2233+
"""Each mode entry contains the expected fields."""
2234+
body = self._get_course_details().json()
2235+
mode = body['course_modes'][0]
2236+
for field in ('slug', 'name', 'min_price', 'suggested_prices', 'currency',
2237+
'expiration_datetime', 'description', 'sku', 'bulk_sku'):
2238+
assert field in mode, f"Missing course_mode field: {field}"
2239+
2240+
def test_invalid_course_id_returns_400(self):
2241+
resp = self._get_course_details(course_id='not/a/real/course')
2242+
assert resp.status_code == status.HTTP_400_BAD_REQUEST
2243+
2244+
def test_nonexistent_course_returns_400(self):
2245+
resp = self._get_course_details(course_id='course-v1:Org+NonExistent+2099')
2246+
assert resp.status_code == status.HTTP_400_BAD_REQUEST
2247+
2248+
2249+
@skip_unless_lms
2250+
class EnrollmentListViewResponseShapeTest(ModuleStoreTestCase, APITestCase):
2251+
"""
2252+
Tests that verify EnrollmentListView (GET /enrollment/v1/enrollment)
2253+
response structure is preserved after migrating to CourseEnrollmentSerializer + ORM (ADR 0025).
2254+
"""
2255+
2256+
USERNAME = "TestLearner"
2257+
PASSWORD = "edx"
2258+
2259+
def setUp(self):
2260+
super().setUp()
2261+
self.course = CourseFactory.create(emit_signals=True)
2262+
CourseModeFactory.create(
2263+
course_id=self.course.id,
2264+
mode_slug=CourseMode.DEFAULT_MODE_SLUG,
2265+
mode_display_name=CourseMode.DEFAULT_MODE_SLUG,
2266+
)
2267+
self.user = UserFactory.create(username=self.USERNAME, password=self.PASSWORD)
2268+
self.client.login(username=self.USERNAME, password=self.PASSWORD)
2269+
CourseEnrollment.enroll(self.user, self.course.id)
2270+
2271+
def _get_enrollments(self, user=None):
2272+
url = reverse('courseenrollments')
2273+
if user:
2274+
url += f'?user={user}'
2275+
return self.client.get(url)
2276+
2277+
def test_returns_200(self):
2278+
assert self._get_enrollments().status_code == status.HTTP_200_OK
2279+
2280+
def test_response_is_list(self):
2281+
body = self._get_enrollments().json()
2282+
assert isinstance(body, list)
2283+
2284+
def test_enrollment_top_level_fields(self):
2285+
"""Each enrollment entry contains the expected top-level fields."""
2286+
body = self._get_enrollments().json()
2287+
assert len(body) >= 1
2288+
entry = body[0]
2289+
for field in ('created', 'mode', 'is_active', 'user', 'course_details'):
2290+
assert field in entry, f"Missing top-level field: {field}"
2291+
2292+
def test_enrollment_user_and_mode_values(self):
2293+
body = self._get_enrollments().json()
2294+
entry = body[0]
2295+
assert entry['user'] == self.USERNAME
2296+
assert entry['mode'] == CourseMode.DEFAULT_MODE_SLUG
2297+
assert entry['is_active'] is True
2298+
2299+
def test_enrollment_course_details_fields(self):
2300+
"""course_details nested object contains the expected fields."""
2301+
body = self._get_enrollments().json()
2302+
course_details = body[0]['course_details']
2303+
for field in ('course_id', 'course_name', 'enrollment_start', 'enrollment_end',
2304+
'course_start', 'course_end', 'invite_only', 'course_modes'):
2305+
assert field in course_details, f"Missing course_details field: {field}"
2306+
2307+
def test_no_enrollments_returns_empty_list(self):
2308+
"""A user with no enrollments gets an empty list, not null or an error."""
2309+
new_user = UserFactory.create(password=self.PASSWORD)
2310+
self.client.login(username=new_user.username, password=self.PASSWORD)
2311+
body = self.client.get(reverse('courseenrollments')).json()
2312+
assert body == []
2313+
2314+
2315+
@skip_unless_lms
2316+
class UserRoleViewResponseShapeTest(ModuleStoreTestCase):
2317+
"""
2318+
Tests that verify EnrollmentUserRolesView (GET /enrollment/v1/roles/)
2319+
response structure is preserved after migrating to UserRolesResponseSerializer (ADR 0025).
2320+
"""
2321+
2322+
USERNAME = "RoleTester"
2323+
PASSWORD = "edx"
2324+
2325+
def setUp(self):
2326+
super().setUp()
2327+
self.course = CourseFactory.create(emit_signals=True, org="testorg", course="c1", run="r1")
2328+
self.user = UserFactory.create(username=self.USERNAME, password=self.PASSWORD)
2329+
self.client.login(username=self.USERNAME, password=self.PASSWORD)
2330+
2331+
def _get_roles(self, course_id=None):
2332+
url = reverse('roles')
2333+
if course_id:
2334+
url += f'?course_id={course_id}'
2335+
return self.client.get(url)
2336+
2337+
def test_returns_200(self):
2338+
assert self._get_roles().status_code == status.HTTP_200_OK
2339+
2340+
def test_response_top_level_keys(self):
2341+
"""Response always contains 'roles' (list) and 'is_staff' (bool)."""
2342+
body = self._get_roles().json()
2343+
assert 'roles' in body
2344+
assert 'is_staff' in body
2345+
assert isinstance(body['roles'], list)
2346+
assert isinstance(body['is_staff'], bool)
2347+
2348+
def test_no_roles_returns_empty_list(self):
2349+
body = self._get_roles().json()
2350+
assert body['roles'] == []
2351+
assert body['is_staff'] is False
2352+
2353+
def test_role_entry_shape(self):
2354+
"""A role entry contains org, course_id, and role fields."""
2355+
role = CourseStaffRole(self.course.id)
2356+
role.add_users(self.user)
2357+
body = self._get_roles().json()
2358+
assert len(body['roles']) == 1
2359+
entry = body['roles'][0]
2360+
for field in ('org', 'course_id', 'role'):
2361+
assert field in entry, f"Missing role field: {field}"
2362+
assert entry['org'] == self.course.org
2363+
assert entry['course_id'] == str(self.course.id)
2364+
2365+
def test_is_staff_true_for_staff_user(self):
2366+
staff_user = UserFactory.create(password=self.PASSWORD, is_staff=True)
2367+
self.client.login(username=staff_user.username, password=self.PASSWORD)
2368+
body = self._get_roles().json()
2369+
assert body['is_staff'] is True
2370+
2371+
def test_filter_by_course_id(self):
2372+
"""course_id query param filters roles to that course only."""
2373+
course2 = CourseFactory.create(emit_signals=True, org="other", course="c2", run="r2")
2374+
CourseStaffRole(self.course.id).add_users(self.user)
2375+
CourseStaffRole(course2.id).add_users(self.user)
2376+
body = self._get_roles(course_id=str(self.course.id)).json()
2377+
assert all(r['course_id'] == str(self.course.id) for r in body['roles'])

0 commit comments

Comments
 (0)