Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#14158 closed (fixed)

SelectMultiple _has_changed assumes data and inital are in same order

Reported by: akaariai 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 akaariai 5 years ago.
14158.diff (1.2 KB) - added by dmoisset 5 years ago.

Download all attachments as: .zip

Change History (6)

Changed 5 years ago by akaariai

comment:1 Changed 5 years ago by dmoisset

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Looks like a legitimate bug

Changed 5 years ago by dmoisset

comment:2 Changed 5 years ago by dmoisset

  • Patch needs improvement unset
  • Triage Stage changed from Accepted to Ready for checkin

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

comment:3 Changed 5 years ago by jezdez

  • Resolution set to fixed
  • Status changed from new to closed

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

comment:4 Changed 5 years ago by jezdez

(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