Skip to content

respecting ordered sequence while partial update is fixed#9902

Open
Natgho wants to merge 1 commit intoencode:mainfrom
Natgho:6202-partial-update-respect-ordered-sequence
Open

respecting ordered sequence while partial update is fixed#9902
Natgho wants to merge 1 commit intoencode:mainfrom
Natgho:6202-partial-update-respect-ordered-sequence

Conversation

@Natgho
Copy link
Contributor

@Natgho Natgho commented Feb 25, 2026

refs #6202

@Natgho
Copy link
Contributor Author

Natgho commented Feb 25, 2026

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.

Copy link

@ProstoSerghei ProstoSerghei left a comment

Choose a reason for hiding this comment

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

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:

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

In the new implementation:

  1. Parse indexed HTML list
  2. If found → return
  3. If partial=True → return empty
  4. Then check plain key in dictionary
  5. 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']})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants