Opened 12 years ago

Closed 11 years ago

#18898 closed Bug (fixed)

has_changed fails to compare model instance with primary key

Reported by: adsva Owned by: nobody
Component: Forms Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a ModelForm with a ModelChoiceField that has a model instance as initial data. The form is rendered as an extra form in a formset and empty_permitted is True, but it still goes through validation since has_changed on the widget fails to equate the pk of the pre-selected field as POSTed with the model instance from the initial data. Maybe ModelChoiceField needs its own widget that handles this comparison?

Attachments (1)

0001-added-a-failing-test-case-for-18898.patch (1.5 KB ) - added by tuxcanfly 11 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 by Russell Keith-Magee, 11 years ago

Triage Stage: UnreviewedAccepted

I'm not sure if a separate widget is required; all that is needed is to identify why an initial model is identified as a change in model data.

comment:2 by tuxcanfly, 11 years ago

I've added a test case to reproduce this.

A workaround is to pass instance.pk instead of instance to initial.

I'm not sure how a widget can figure out that the initial data was a model instance so that
it can use the primary key while comparing.

I think this issue also affects ModelMultipleChoiceField.

comment:3 by tuxcanfly, 11 years ago

Trying a fix by updating has_changed to check on both string and python values.

Added test for ModelMultipleChoiceField as well. Verified that all tests pass on sqlite.

Needs review: https://github.com/django/django/pull/660

comment:4 by Claude Paroz, 11 years ago

Has patch: set

I did some refactoring of the _has_changed code for #16612, and I included your tests which now also pass.

https://github.com/django/django/pull/676

comment:5 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In 3318595c0bfeda9f6bae8aa11dda68647ae55fde:

Fixed #18898 -- Added tests with a fix for ModelMultipleChoiceField

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