[Django] #27758: Template widget rendering breaks the AdvancedModelIterator pattern
Django
----------------------------------------+------------------------
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
----------------------------------------+------------------------
Occasionally, when extending `ModelChoiceField` and
`ModelMultipleChoiceField` with a custom widget, it is useful for the
widget to know the object used to create the value, label pair. This could
be used to adding custom HTML attributes to very specific choices.
Historically, a pattern was developed to handle this situation named
[http://srcmvn.com/blog/2013/01/15/django-advanced-model-choice-field
AdvancedModelChoiceIterator]. The pattern described in the article breaks
in 1.11.
Firstly, `ChoiceWidget` now always expects a 2-tuple returned by
`ModelChoiceIterator.choice()` as it is unpacked in
[https://github.com/django/django/blob/a15d81a183e2d85969ed46adb975661515330b16/django/forms/widgets.py#L573
ChoiceWidget.optgroups()]. The usage is
[https://docs.djangoproject.com/en/1.10/ref/forms/fields/#django.forms.ChoiceField.choices
documented] as requiring a 2-tuple, so maybe this is considered OK.
An alternative I considered was returning a wrapped value to hold the
object, hoping this would be accessible from
`ChoiceWidget.create_option()`. Unfortunately this did not work as the
value is eagerly coerced to a string very early in
[https://github.com/django/django/blob/a15d81a183e2d85969ed46adb975661515330b16/django/forms/widgets.py#L577
ChoiceWidget.optgroups()]. For example:
{{{
class AdvancedChoiceValue:
def __init__(self, value, obj):
self.value = value
self.obj = obj
def __str__(self):
return str(self.value)
class AdvancedChoiceIterator(ModelChoiceIterator):
def choice(self, obj):
value, label = super().choice(obj)
return AdvancedChoiceValue(value, obj), label
}}}
I think delaying the `force_text()` until checking the selected value
would solve this problem.
--
Ticket URL: <https://code.djangoproject.com/ticket/27758>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* has_patch: 0 => 1
* needs_tests: 0 => 1
Comment:
[https://github.com/django/django/pull/7924 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:1>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Jon Dufresne):
* needs_tests: 1 => 0
Comment:
Added test.
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:2>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* severity: Normal => Release blocker
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:3>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Jon Dufresne):
I should say that I'm very open to working towards an alternative solution
if someone sees a better opportunity to solve the same problem in a more
elegant way.
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:4>
Django
Reporter: Jon Dufresne | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.11
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* cc: Preston Timmons (added)
Comment:
The proposed patch seem sensible given the current APIs. In the long run,
it could be nice to consider a change that allows this pattern without so
much boilerplate. Any other ideas, Preston?
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:5>
Django
Reporter: Jon Dufresne | Owner: nobody
Component: Forms | Version: 1.11
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"6d8979f4c2fbfb9fd5db92acd72489cbbcbdd5d1" 6d8979f4]:
{{{
#!CommitTicketReference repository=""
revision="6d8979f4c2fbfb9fd5db92acd72489cbbcbdd5d1"
Fixed #27758 -- Reallowed AdvancedModelIterator pattern after template
widget rendering.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:6>
Django
Reporter: Jon Dufresne | Owner: nobody
Component: Forms | Version: 1.11
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"52e9c1c8b72bc30a66d838187bd9ecb1f22f8448" 52e9c1c]:
{{{
#!CommitTicketReference repository=""
revision="52e9c1c8b72bc30a66d838187bd9ecb1f22f8448"
[1.11.x] Fixed #27758 -- Reallowed AdvancedModelIterator pattern after
template widget rendering.
Backport of 6d8979f4c2fbfb9fd5db92acd72489cbbcbdd5d1 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/27758#comment:7>