#27758 closed Bug (fixed)
Template widget rendering breaks the AdvancedModelIterator pattern
Reported by: | Jon Dufresne | Owned by: | nobody |
---|---|---|---|
Component: | Forms | Version: | 1.11 |
Severity: | Release blocker | Keywords: | |
Cc: | Preston Timmons | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
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 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 ChoiceWidget.optgroups(). The usage is 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 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.
Change History (7)
comment:1 by , 8 years ago
Has patch: | set |
---|---|
Needs tests: | set |
comment:3 by , 8 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:4 by , 8 years ago
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.
comment:5 by , 8 years ago
Cc: | added |
---|
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?
PR