Opened 14 years ago

Closed 14 years ago

Last modified 14 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: no UI/UX: no

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 14 years ago.
14158.diff (1.2 KB ) - added by Daniel F Moisset 14 years ago.

Download all attachments as: .zip

Change History (6)

by Anssi Kääriäinen, 14 years ago

Attachment: sm_has_changed.diff added

comment:1 by Daniel F Moisset, 14 years ago

Patch needs improvement: set
Triage Stage: UnreviewedAccepted

Looks like a legitimate bug

by Daniel F Moisset, 14 years ago

Attachment: 14158.diff added

comment:2 by Daniel F Moisset, 14 years ago

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 by Jannis Leidel, 14 years ago

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 by Jannis Leidel, 14 years ago

(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