Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#23250 closed Cleanup/optimization (fixed)

Document that ModelMultipleChoiceField queryset may be None

Reported by: wraus Owned by: nobody
Component: Documentation Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a ModelMultipleChoiceField whose queryset I want to populate after the initialization of the form.

class FooMultipleChoiceForm(forms.Form)
    foo_select = forms.ModelMultipleChoiceField(
        label="Foo Selection",
        help_text="Select a Foo from the choices above.",
        queryset=None,
    )

    def __init__(self, bar, *args, **kwargs):
        super(FooMultipleChoiceForm, self).__init__(*args, **kwargs)
        self.fields['foo_select'].queryset = bar.foo_set.all()

Currently, there are two major issues with this:

  1. Even if I want to set the queryset later in __init__ with self.fields['foo_select'], I still have to specify a queryset attribute when declaring the field, even though the given queryset will be overwritten in __init__. This not only seems redundant, but it feels odd to be forced to enter a value for the explicit purpose of having it overwritten later, especially when providing None for the queryset attribute doesn't cause any issues if a new queryset is provided in the __init__ of the form.
  2. If I omit the queryset attribute in ModelMultipleChoiceField, I get a confusing error message:
    TypeError: __init__() takes at least 2 arguments (3 given)
    
    Nothing is mentioned about the missing queryset argument, and given that I'm setting it later, it's easy to overlook that it's the cause of the problem.

Given that setting queryset to None doesn't cause problems as long as the queryset is initialized later in the form, it seems like it might make sense to allow queryset to be unspecified at field declaration time, as long as it's specified later.

Attachments (1)

23250.diff (984 bytes) - added by Tim Graham 2 years ago.

Download all attachments as: .zip

Change History (6)

comment:1 Changed 2 years ago by Tim Graham

Component: FormsDocumentation
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

I have used this pattern before as well, however, I think it would be best to document that queryset may be None if you want to specify it later. Making it optional seems likely to trip up newbies.

comment:2 Changed 2 years ago by Tim Graham

Has patch: set
Summary: Improve Error Message / Handling for ModelMultipleChoiceField and queryset attributeDocument that ModelMultipleChoiceField queryset may be None

Changed 2 years ago by Tim Graham

Attachment: 23250.diff added

comment:3 Changed 2 years ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In ced3e303ca6c902c14d08b68806230ce04c71ff7:

Fixed #23250 -- Documented that ModelMultipleChoiceField queryset may be None.

comment:4 Changed 2 years ago by Tim Graham <timograham@…>

In 8e77ac634ffcbeab634aa8c3f1bda7e6fab229a0:

[1.6.x] Fixed #23250 -- Documented that ModelMultipleChoiceField queryset may be None.

Backport of ced3e303ca from master

comment:5 Changed 2 years ago by Tim Graham <timograham@…>

In 1b89f976f4919a36c6b70eba428c3a02a94e2822:

[1.7.x] Fixed #23250 -- Documented that ModelMultipleChoiceField queryset may be None.

Backport of ced3e303ca from master

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