[Django] #22786: value_from_datadict gets called twice (additionally in changeform_view)
Django
-------------------------------+--------------------
Reporter: blueyed | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: master
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
I've noticed that a widget's `value_from_datadict` gets called twice when
changing a model in the admin, which could and
should probably be avoided to save extra processing, especially since the
second call is only for `construct_change_message`.
While there might be use cases for this (which?), it seems like
`value_from_datadict` is not meant to return a different result on the
second call anyway?!
If this is the desired behavior, it should be documented, and a
recommendation being added to cache any extra processing (like downloading
external files).
{{{
…/django-1.7.x/django/forms/forms.py(353)full_clean()
351 return
352
--> 353 self._clean_fields()
354 self._clean_form()
355 self._post_clean()
…/django-1.7.x/django/forms/forms.py(362)_clean_fields()
360 # Each widget type knows how to retrieve its own data,
because some
361 # widgets split data over several HTML fields.
--> 362 value = field.widget.value_from_datadict(self.data,
self.files, self.add_prefix(name))
363 try:
364 if isinstance(field, FileField):
> …/project/dh_utils/forms.py(123)value_from_datadict()
121 def value_from_datadict(self, data, files, name):
122 import ipdb; ipdb.set_trace()
--> 123 return None
124 upload = super(ClearableURLFileInput,
self).value_from_datadict(data, files, name)
125 if not self.is_required and
CheckboxInput().value_from_datadict(
}}}
{{{
…/django-1.7.x/django/contrib/admin/options.py(1369)changeform_view()
1367 return self.response_add(request, new_object)
1368 else:
-> 1369 change_message =
self.construct_change_message(request, form, formsets)
1370 self.log_change(request, new_object,
change_message)
1371 return self.response_change(request,
new_object)
…/django-1.7.x/django/contrib/admin/options.py(951)construct_change_message()
949 """
950 change_message = []
--> 951 if form.changed_data:
952 change_message.append(_('Changed %s.') %
get_text_list(form.changed_data, _('and')))
953
…/django-1.7.x/django/forms/forms.py(419)changed_data()
417 for name, field in self.fields.items():
418 prefixed_name = self.add_prefix(name)
--> 419 data_value =
field.widget.value_from_datadict(self.data, self.files, prefixed_name)
420 if not field.show_hidden_initial:
421 initial_value = self.initial.get(name,
field.initial)
> …/project/dh_utils/forms.py(123)value_from_datadict()
121 def value_from_datadict(self, data, files, name):
122 import ipdb; ipdb.set_trace()
--> 123 return None
124 upload = super(ClearableURLFileInput,
self).value_from_datadict(data, files, name)
125 if not self.is_required and
CheckboxInput().value_from_datadict(
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22786>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: blueyed | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
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 timo):
* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
Haven't looked into whether the second call can be avoided, but accepting
for documentation if not.
--
Ticket URL: <https://code.djangoproject.com/ticket/22786#comment:1>
Django
Reporter: blueyed | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.admin | Version: master
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
Comment:
Most of the time, `value_from_datadict` is just retrieving some content
from a dictionary with light processing, so I don't think it would justify
caching its result. Attached a documentation proposal.
--
Ticket URL: <https://code.djangoproject.com/ticket/22786#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: blueyed | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: Documentation | Resolution:
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: contrib.admin => Documentation
* stage: Accepted => Ready for checkin
Comment:
Might replace "costly procedures" with "expensive processing" which
appears elsewhere in the docs.
--
Ticket URL: <https://code.djangoproject.com/ticket/22786#comment:3>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | 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:"909015a51b98749cabb0d13f8ea0bf58ec707fa3"]:
{{{
#!CommitTicketReference repository=""
revision="909015a51b98749cabb0d13f8ea0bf58ec707fa3"
Fixed #22786 -- Documented value_from_datadict caveat
Thanks blueyed for the report and Tim Graham for the review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22786#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Documentation | Resolution: fixed
Severity: Normal | Triage Stage: Ready for
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Claude Paroz <claude@…>):
In [changeset:"022fdb2ac4a795af0fb1808942b260e2dc3d404d"]:
{{{
#!CommitTicketReference repository=""
revision="022fdb2ac4a795af0fb1808942b260e2dc3d404d"
[1.7.x] Fixed #22786 -- Documented value_from_datadict caveat
Thanks blueyed for the report and Tim Graham for the review.
Backport of 909015a51b from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/22786#comment:5>