Opened 4 years ago

Closed 4 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 4 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 4 years ago by Russell Keith-Magee

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
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.

Changed 4 years ago by tuxcanfly

comment:2 Changed 4 years ago by tuxcanfly

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 Changed 4 years ago by tuxcanfly

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 Changed 4 years ago by Claude Paroz

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 Changed 4 years ago by Claude Paroz <claude@…>

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