Opened 10 months ago

Closed 9 months ago

Last modified 9 months ago

#31155 closed Bug (fixed)

Named groups in choices are not properly validated in case of non str typed values.

Reported by: sakkada Owned by: Mariusz Felisiak
Component: Database layer (models, ORM) Version: 3.0
Severity: Release blocker Keywords: choices
Cc: Nick Pope Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In case of using typed choices and string value to store it (in my case it is multiple values stored in char field as JSON) it is possible to catch error while run makemigrations (_check_choices error):

main.MultiValueFieldModel.multi_value_field_integer_with_grouped_choices: (fields.E005) 'choices' must be an iterable containing (actual value, human readable name) tuples.

Looking deeper into the django code, we see actual error message: 'int' object is not iterable and it happens in this block of code (https://github.com/django/django/blob/aa6c620249bc8c2a6245c8d7b928b05e7e5e78fc/django/db/models/fields/__init__.py#L271-L275):

if self.max_length is not None and group_choices:
    choice_max_length = max(
        choice_max_length,
        *(len(value) for value, _ in group_choices if isinstance(value, str)),
    )

If we have CharField (any other with max_length defined) and grouped choices with non str typed values like:

choices=(
    ('one', ((1, 'One',), (11, 'Eleven',),),),
    ('two', ((2, 'Two',), (22, 'Twenty two',),),),
)

we will have the situation, when max function receives only one integer value (choice_max_length), because (len(value) for value, _ in group_choices if isinstance(value, str)) will return empty generator, and that is why error 'int' object is not iterable raises (max function waits iterable if there is only one argument).

Code block:

choice_max_length = max(
    choice_max_length,
    *(len(value) for value, _ in group_choices if isinstance(value, str)),
)

in this case works like:

choice_max_length = max(
    choice_max_length,
    *[],
)

which is incorrect.

The simples solution is to add one additional argument to max function, which will be usefull only in this partucular situation:

choice_max_length = max(
    choice_max_length, 0,
    *(len(value) for value, _ in group_choices if isinstance(value, str)),
)

Change History (5)

comment:1 Changed 9 months ago by Mariusz Felisiak

Owner: changed from nobody to Mariusz Felisiak
Severity: NormalRelease blocker
Status: newassigned
Triage Stage: UnreviewedAccepted

Thanks for this report. Regression in b6251956b69512bf230322bd7a49b629ca8455c6.

comment:2 Changed 9 months ago by Mariusz Felisiak

Cc: Nick Pope added
Has patch: set

comment:3 Changed 9 months ago by Claude Paroz

Triage Stage: AcceptedReady for checkin

comment:4 Changed 9 months ago by GitHub <noreply@…>

Resolution: fixed
Status: assignedclosed

In 6f7998ad:

Fixed #31155 -- Fixed a system check for the longest choice when a named group contains only non-string values.

Regression in b6251956b69512bf230322bd7a49b629ca8455c6.

Thanks Murat Guchetl for the report.

comment:5 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 92866682:

[3.0.x] Fixed #31155 -- Fixed a system check for the longest choice when a named group contains only non-string values.

Regression in b6251956b69512bf230322bd7a49b629ca8455c6.

Thanks Murat Guchetl for the report.
Backport of 6f7998adc784032f4b8918ca2eea27537ea4cbbe from master

Note: See TracTickets for help on using tickets.
Back to Top