Opened 5 years ago

Closed 5 years ago

Last modified 5 years 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: pope1ni 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 by Mariusz Felisiak, 5 years ago

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

Thanks for this report. Regression in b6251956b69512bf230322bd7a49b629ca8455c6.

comment:2 by Mariusz Felisiak, 5 years ago

Cc: pope1ni added
Has patch: set

comment:3 by Claude Paroz, 5 years ago

Triage Stage: AcceptedReady for checkin

comment:4 by GitHub <noreply@…>, 5 years ago

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 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

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