[Django] #24428: Model field with default=callable and choices always is always reported at changed
Django
changed
-------------------------------+--------------------
Reporter: CarstenF | Owner: nobody
Type: Bug | Status: new
Component: Uncategorized | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
Originating thread at django-users:
https://groups.google.com/forum/#!topic/django-users/WhuG7X9eRWk
In a newly created Django project/app, please consider this `models.py`
file:
{{{
#!python
# -*- coding: utf-8 -*-
from django.db import models
def VormonatsMonat():
return 1
MONTHS_CHOICES = (
(1, "Jan"),
(2, "Feb"),
)
class Choice(models.Model):
choice_text = models.CharField(max_length=200)
votes = models.IntegerField(default=0)
monat = models.IntegerField(default=VormonatsMonat,
choices=MONTHS_CHOICES)
}}}
Then, at the `./manage.py shell` prompt (creating a Choice instance is
omitted here):
{{{
#!python
>>> from django.forms import ModelForm
>>> from TestApp.models import Choice
>>> class ChoiceForm(ModelForm):
... class Meta:
... model = Choice
... fields = ["choice_text", "votes", "monat"]
...
>>> ch = Choice.objects.get(pk=1)
>>> chv = Choice.objects.values().get(pk=1)
>>> ch
<Choice: Choice object>
>>> chv
{'monat': 1, 'choice_text': u'just a test', 'votes': 0, u'id': 1}
>>> ChoiceForm(chv, initial=chv, instance=ch).has_changed()
True
}}}
The expected output is `False`, which is (in a new shell session) in fact
obtained whenever the definition of field `monat` is slightly changed,
e.g. to a constant default value rather than a callable, or with no
choices.
I got as far as `BaseForm.changed_data()` in `django/forms/forms.py`,
where for field `monat` we get
- `field.show_hidden_initial == True`,
- `hidden_widget.value_from_datadict(self.data, self.files,
initial_prefixed_name) == None`,
- resulting in `initial_value == u""`,
resulting in the observed behaviour.
Is maybe `field.show_hidden_initial == True` (wrongly) because the
`default=callable` makes it seem that the default value is not one of the
available choices?
--
Ticket URL: <https://code.djangoproject.com/ticket/24428>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
changed
Reporter: CarstenF | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_docs: => 0
* component: Uncategorized => Forms
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
`show_hidden_initial = True` is correct because we need to know the
initially selected choice. It seems the issue is in `Field.has_changed()`
where the initial value is compared as a string to the coerced value as an
integer. I think it's a bug.
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:1>
Django
--------------------------+------------------------------------
Reporter: CarstenF | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:2>
Django
--------------------------+------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by CarstenF):
Looking at Django 1.7.5's `BaseForm.changed_data()`, debugging indicates
that line
{{{
#!python
elif field._has_changed(initial_value, data_value):
}}}
for field `monat` of my above example turns into
{{{
#!python
elif field._has_changed(u"", 1):
}}}
Thus, afaics, it rather seems that `initial_value` is wrongly determined
earlier, when call `hidden_widget.value_from_datadict(self.data,
self.files, initial_prefixed_name)` returns `None` rather than 1.
`self.data` in turn has no key `initial_prefixed_name` (here "initial-
monat"), causing the `None` return value above.
What I wonder about is how and where "initial-monat" should have been
added to `self.data` in the first place...?
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:3>
Django
--------------------------+------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
It's a hidden field that appears when the form is rendered. If you are
testing in Python only, you should include that field in the form data
yourself.
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:4>
Django
--------------------------+------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by claudep):
* has_patch: 0 => 1
* version: 1.7 => master
Comment:
https://github.com/django/django/pull/4237
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:5>
Django
--------------------------+------------------------------------
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by CarstenF):
Gee, Tim and Claude, many thanks!!
Just when I was reading the contributor's docs to attempt a patch myself.
;-) But I would probably have not been able to come up with the proper
tests anyway (well, with a bit of luck, next time), so I'm very happy you
did!
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:6>
Django
Reporter: CarstenF | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:7>
Django
--------------------------+---------------------------------------------
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Claude Paroz <claude@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"8714403614c4dfa37e806db4a6708b8a91a827a4"]:
{{{
#!CommitTicketReference repository=""
revision="8714403614c4dfa37e806db4a6708b8a91a827a4"
Fixed #24428 -- Fixed has_changed for fields with coercion
Thanks Carsten Fuchs for the report.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24428#comment:8>