Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#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 Jon Dufresne, 8 years ago

Has patch: set
Needs tests: set

comment:2 by Jon Dufresne, 8 years ago

Needs tests: unset

Added test.

comment:3 by Tim Graham, 8 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

comment:4 by Jon Dufresne, 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 Tim Graham, 8 years ago

Cc: Preston Timmons 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?

comment:6 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: newclosed

In 6d8979f4:

Fixed #27758 -- Reallowed AdvancedModelIterator pattern after template widget rendering.

comment:7 by Tim Graham <timograham@…>, 8 years ago

In 52e9c1c:

[1.11.x] Fixed #27758 -- Reallowed AdvancedModelIterator pattern after template widget rendering.

Backport of 6d8979f4c2fbfb9fd5db92acd72489cbbcbdd5d1 from master

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