respecting ordered sequence while partial update is fixed#9902
respecting ordered sequence while partial update is fixed#9902Natgho wants to merge 1 commit intoencode:mainfrom
Conversation
|
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
left a comment
There was a problem hiding this comment.
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=Truesemantics when the field is absent
In the new implementation:
- Parse indexed HTML list
- If found → return
- If
partial=True→ returnempty - 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
fieldandfield[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
withpartial=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=Truewith non-indexed list input:data = MultiValueDict({'colors': ['#ffffff', '#000000']})
refs #6202