Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#14158 closed (fixed)

SelectMultiple _has_changed assumes data and inital are in same order

Reported by: Anssi Kääriäinen Owned by: nobody
Component: Forms Version: 1.2
Severity: Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

The _has_changed method assumes data and initial are in same order:

for value1, value2 in zip(initial, data):
    if force_unicode(value1) != force_unicode(value2):
        return True

This seems like a dangerous assumption, because initial and data might come from different sources. For example if you override the queryset for ModelMultipleChoiceField to have custom ordering, initial and data returned from the HTML form will probably differ in order. And by default m2m initial data in model forms is retrieved without any ordering, so it is just assumed that every time the query is run, the data will be retrieved in the same order. This is of course wrong.

Attachments (2)

sm_has_changed.diff (1.2 KB) - added by Anssi Kääriäinen 6 years ago.
14158.diff (1.2 KB) - added by dmoisset 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by Anssi Kääriäinen

Attachment: sm_has_changed.diff added

comment:1 Changed 6 years ago by dmoisset

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Looks like a legitimate bug

Changed 6 years ago by dmoisset

Attachment: 14158.diff added

comment:2 Changed 6 years ago by dmoisset

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

Added an attachment based on akaariai's, but with a more pythonic way to compare the sets.

comment:3 Changed 6 years ago by Jannis Leidel

Resolution: fixed
Status: newclosed

(In [14887]) Fixed #14158 -- Fixed SelectMultiple._has_changed to not assume same order of data anymore. Thanks, akaariai and dmoisset.

comment:4 Changed 6 years ago by Jannis Leidel

(In [14907]) [1.2.X] Fixed #14158 -- Fixed SelectMultiple._has_changed to not assume same order of data anymore. Thanks, akaariai and dmoisset.

Backport from trunk (r14887).

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