openedx / openedx/openedx-platform

Un-pin djangorestframework (currently <3.18): 3.18 changes many=True error response shape

Open
#39,048 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
8.2k
Forks
4.4k
Avg merge
6d 18h
Merged PRs (30d)
42

Description

djangorestframework is pinned to <3.18 in [tool.edx_lint].uv_constraints. This issue tracks the work needed to remove that pin.

Why it's pinned

DRF 3.18.0 includes encode/django-rest-framework#9837, which changes how ListSerializer (i.e. many=True) reports child validation errors. Previously the errors were a list padded with empty dicts, one slot per input item. Now they are a dict keyed by the index of each failing item, and passing items are omitted entirely:

# 3.17.1
errors = []
errors.append(exc.detail)   # for a failing item
errors.append({})           # for a passing item

# 3.18.0
errors = {}
errors[index] = exc.detail  # failing items only

This changes the JSON type of the error response body from an array to an object. Given:

class Item(serializers.Serializer):
    code = serializers.CharField()

Item(data=[{"code": "ok"}, {"bad": 1}, {"code": "fine"}], many=True)

the rendered 400 body changes as follows:

Version Response body
3.17.1 [{},{"code":["This field is required."]},{}]
3.18.0 {"1":{"code":["This field is required."]}}

Nested many=True fields change the same way:

3.17.1  {"items":[{},{"code":["This field is required."]}]}
3.18.0  {"items":{"1":{"code":["This field is required."]}}}

Note that the index becomes a string key once serialized to JSON, and that clients doing errors[0], errors.length, or iterating the array will break.

Affected endpoints

These call is_valid(raise_exception=True) on a many=True serializer bound to request.data, so the errors go straight to the client via DRF's default exception handler (this repo does not configure a custom EXCEPTION_HANDLER):

Endpoint Code
POST/PATCH/PUT /api/program_enrollments/v1/programs/{uuid}/enrollments/ lms/djangoapps/program_enrollments/rest_api/v1/views.py:98
POST/PATCH/PUT /api/program_enrollments/v1/programs/{uuid}/courses/{course_id}/enrollments/ same handle_write_request
POST /api/contentstore/v0/tabs/{course_id}/reorder cms/djangoapps/contentstore/rest_api/v0/views/tabs.py:249
cms course-runs write (course team) cms/djangoapps/api/v1/serializers/course_runs.py:52

Each of these responses is part of an API contract, so all of them need consideration regardless of who the known callers are. Partial-failure batches are a normal mode of operation for the program-enrollments endpoints in particular, which makes the array-to-object flip especially likely to be observed there.

This list was compiled by auditing the explicit data=..., many=True write paths. There are roughly 90 nested SubSerializer(many=True) field declarations in the codebase; most are response-only serializers and were not individually audited, so the table above should be treated as confirmed but not necessarily exhaustive.

Known test impact

Only one test in the repo pinned this format. Under 3.18.0, three cases of test_patch_invalid_language_proficiencies fail:

openedx/core/djangoapps/user_api/accounts/tests/test_views.py::TestAccountsAPI::test_patch_invalid_language_proficiencies_2
openedx/core/djangoapps/user_api/accounts/tests/test_views.py::TestAccountsAPI::test_patch_invalid_language_proficiencies_3
openedx/core/djangoapps/user_api/accounts/tests/test_views.py::TestAccountsAPI::test_patch_invalid_language_proficiencies_4
- 'language_proficiencies': [{'code': ['This field is required.']}]
+ 'language_proficiencies': {0: {'code': ['This field is required.']}}

The accounts API itself is not a structural break: add_serializer_errors in openedx/core/lib/api/view_utils.py stringifies these errors into the free-form developer_message, so only the text inside that string changes. The four endpoints in the table above are the ones whose response structure actually changes.

Note that the other four endpoints have no tests asserting their error bodies, which is why they changed silently and CI stayed green for them.

Work needed to un-pin

  • Update the three test_patch_invalid_language_proficiencies expectations to the dict-keyed shape.
  • Decide whether the new error shape is acceptable for the affected endpoints, or whether the platform should normalize it back to a list.
    • Note: normalizing back to a list is not a trivial mapping. The new dict is sparse (passing items are omitted), so flattening it to a list would misattribute errors to the wrong index. Preserving the old format would require re-padding using the original input length.
  • Notify API consumers before changing these error contracts, and treat the change as breaking regardless of which callers are currently known.
  • Add release-note / breaking-change documentation for whatever shape is settled on.
  • Consider adding test coverage for the error bodies of the four endpoints above, so a future change to this contract is not silent.

Context

The pin was added while upgrading Python requirements so that a routine chore: dependency bump would not silently change four API error contracts. See https://github.com/openedx/openedx-platform/pull/39028.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the djangorestframework constraint in [tool.edx_lint].uv_constraints, the three test_patch_invalid_language_proficiencies cases in openedx/core/djangoapps/user_api/accounts/tests/test_views.py, and the listed endpoint entry points. Run those tests, then assess the four affected API contracts and whether to preserve or adopt the new sparse error shape. Done means the dependency can be unpinned with updated tests, consumer communication, release-note documentation, and coverage for the affected responses.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend, documentation, testing
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.