Opened 3 years ago

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

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years ago by russellm

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

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

comment:2 Changed 2 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 2 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 2 years ago by claudep

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

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

In 3318595c0bfeda9f6bae8aa11dda68647ae55fde:

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

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