respecting ordered sequence while partial update is fixed by Natgho · Pull Request #9902 · encode/django-rest-framework

Natgho · 2026-02-25T07:08:43Z

Natgho · 2026-02-25T07:10:04Z

I considered including the ticket ID in the Description section of the test section, but decided it would be excessive. If you want it included, just let me know.

ProstoSerghei

Review: ListField.get_value partial + indexed keys handling

Summary

This PR modifies ListField.get_value() to correctly handle indexed HTML form keys
(e.g. field[0], field[1]) during partial=True updates and adds a regression test
covering that scenario.

The problem being addressed is valid, and the added test demonstrates a real edge case.
However, the change alters the evaluation order inside get_value() in a way that may
impact backward compatibility and deserves closer scrutiny.

1. Behavioral Changes

Previously, the logic flow was effectively:

If HTML input → attempt parse_html_list
Otherwise → fall back to dictionary.get(self.field_name, empty)
Respect partial=True semantics when the field is absent

In the new implementation:

Parse indexed HTML list
If found → return
If partial=True → return empty
Then check plain key in dictionary
Finally return fallback

This reorders evaluation and introduces earlier short-circuit returns.

Concern

For partial=True, the method may now return empty earlier than before,
preventing fallback to dictionary.get(self.field_name, empty) in some cases.

Please clarify whether this preserves behavior for:

Non-indexed list submission in multipart/form-data
Standard HTML form submission without indexed keys
Mixed cases (both field and field[0] present)

2. Partial Update Semantics

The new logic explicitly prioritizes indexed HTML keys before evaluating
plain field names and partial fallback.

Questions:

Was the previous fallback behavior during partial updates considered incorrect?
Have you verified no regression occurs when a non-indexed list is submitted
with partial=True?

Given that get_value() is central to serializer input processing,
any reordering of conditionals should be validated carefully.

3. Test Coverage

The added regression test correctly validates:

Indexed keys (colors[0], colors[1])
partial=True
Preservation of ordering

However, additional coverage would strengthen confidence:

Suggested additional tests

partial=True with non-indexed list input:

data = MultiValueDict({'colors': ['#ffffff', '#000000']})

Copilot

Pull request overview

Fixes ListField handling of indexed HTML form keys (e.g., colors[0], colors[1]) during partial updates so that list values are recognized and ordered correctly, addressing #6202.

Changes:

Update ListField.get_value() to parse indexed HTML keys even when partial=True.
Add regression/backward-compatibility tests covering partial and non-partial HTML list submissions (indexed and non-indexed).
Ignore JetBrains .idea directory in .gitignore.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`rest_framework/fields.py`	Adjusts `ListField.get_value` HTML-input handling to support indexed keys in partial updates.
`tests/test_serializer_lists.py`	Adds regression tests for indexed list keys in partial updates and related compatibility scenarios.
`.gitignore`	Adds `.idea` to ignored files.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rest_framework/fields.py

… keys is improved

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_serializer_lists.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Natgho · 2026-03-02T11:56:04Z

The issues have been resolved and the PR looks ready. Is there anything else that needs improvement?

auvipy · 2026-03-02T15:12:46Z

lets wait for final rounds of reviews

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rest_framework/fields.py

tests/test_fields.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_serializer_lists.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_serializer_lists.py

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rest_framework/fields.py

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tests/test_serializer_lists.py

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Natgho · 2026-03-02T16:46:59Z

Finally!
Sometimes I wish I had been born in the years before AI was created...

Natgho · 2026-03-05T13:30:56Z

@auvipy what do you think?

respecting ordered sequence while partial update is fixed

1544faf

Natgho mentioned this pull request Feb 25, 2026

ListField does not respect ordered sequence in form data with partial updates. #6202

Open

6 tasks

ProstoSerghei reviewed Feb 27, 2026

View reviewed changes

Natgho and others added 4 commits March 2, 2026 12:20

Merge branch 'main' into 6202-partial-update-respect-ordered-sequence

4fdb19c

getting field order is re-ordered for backward compatibility

9caec6e

more detailed tests are added

88ea74c

.idea is added for Pycharm tmp files

0186c25

Natgho requested a review from ProstoSerghei March 2, 2026 09:39

auvipy requested review from auvipy and Copilot and removed request for ProstoSerghei March 2, 2026 10:07

Copilot started reviewing on behalf of auvipy March 2, 2026 10:07 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

rest_framework/fields.py Outdated Show resolved Hide resolved

rest_framework/fields.py Outdated Show resolved Hide resolved

Natgho added 2 commits March 2, 2026 13:31

reduntant check is removed, respecting o(n) search, checking prefixed…

069c9fb

… keys is improved

missing tests are added, more detailed

e1c8863

Natgho requested a review from Copilot March 2, 2026 10:32

Copilot started reviewing on behalf of Natgho March 2, 2026 10:33 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

Update tests/test_serializer_lists.py

1206cc4

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

auvipy requested a review from Copilot March 2, 2026 15:12

Copilot started reviewing on behalf of auvipy March 2, 2026 15:12 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

rest_framework/fields.py Outdated Show resolved Hide resolved

tests/test_fields.py Outdated Show resolved Hide resolved

Natgho and others added 2 commits March 2, 2026 18:25

Update tests/test_fields.py

3bb0f16

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

comment is fixed for current solution explanation

12159c9

Natgho requested a review from Copilot March 2, 2026 15:29

Copilot started reviewing on behalf of Natgho March 2, 2026 15:30 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

Update tests/test_serializer_lists.py

1457bbc

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Natgho requested a review from Copilot March 2, 2026 15:37

Copilot started reviewing on behalf of Natgho March 2, 2026 15:37 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

standardized the errors

b0066ab

Natgho requested a review from Copilot March 2, 2026 16:00

Copilot started reviewing on behalf of Natgho March 2, 2026 16:01 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

rest_framework/fields.py Outdated Show resolved Hide resolved

Update rest_framework/fields.py

acf688b

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Natgho requested a review from Copilot March 2, 2026 16:06

Copilot started reviewing on behalf of Natgho March 2, 2026 16:07 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

tests/test_serializer_lists.py Outdated Show resolved Hide resolved

listfield html input tests are seperated

733c207

Natgho requested a review from Copilot March 2, 2026 16:25

Copilot started reviewing on behalf of Natgho March 2, 2026 16:26 View session

Copilot AI reviewed Mar 2, 2026

View reviewed changes

Natgho and others added 2 commits March 4, 2026 15:45

Merge branch 'main' into 6202-partial-update-respect-ordered-sequence

471ce01

Merge branch 'main' into 6202-partial-update-respect-ordered-sequence

8afbbb2

Uh oh!

Conversation

Natgho commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Natgho commented Feb 25, 2026

Uh oh!

ProstoSerghei left a comment

Choose a reason for hiding this comment

Review: ListField.get_value partial + indexed keys handling

Summary

1. Behavioral Changes

Concern

2. Partial Update Semantics

3. Test Coverage

Suggested additional tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Natgho commented Mar 2, 2026

Uh oh!

auvipy commented Mar 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Natgho commented Mar 2, 2026

Uh oh!

Natgho commented Mar 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Natgho commented Feb 25, 2026 •

edited

Loading