Re: [Django] #2445: [patch] allow callable values for limit_choices_to

[Django]

#2445: [patch] allow callable values for limit_choices_to
---------------------------------------+------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: closed
Milestone: | Component: Core (Other)
Version: | Severity: Normal
Resolution: fixed | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
---------------------------------------+------------------------------
Changes (by anonymous):

* status: new => closed
* resolution: => fixed
* easy: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:37>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#2445: [patch] allow callable values for limit_choices_to
---------------------------------------+------------------------------
Reporter: michael@… | Owner: nobody

Type: New feature | Status: reopened

Milestone: | Component: Core (Other)
Version: | Severity: Normal

Resolution: | Keywords:

Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0
---------------------------------------+------------------------------

Changes (by kmtracey):

* status: closed => reopened
* resolution: fixed =>

Comment:

I don't know if the previous update was a mistake or spam, but this is not
fixed. Using r16279 specifying a callable for limit_choices_to raises a
!TypeError `_filter_or_exclude() argument after ** must be a dictionary`.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:38>

[Django]

#2445: [patch] allow callable values for limit_choices_to
---------------------------------------+------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened
Milestone: | Component: Core (Other)
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0

UI/UX: 0 |
---------------------------------------+------------------------------
Changes (by nih):

* cc: nih (added)
* ui_ux: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:39>

[Django]

#2445: [patch] allow callable values for limit_choices_to
---------------------------------------+------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened
Milestone: | Component: Core (Other)
Version: | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 1
Needs documentation: 1 | Needs tests: 1
Patch needs improvement: 1 | Easy pickings: 0

UI/UX: 1 |
---------------------------------------+------------------------------
Changes (by nih):

* ui_ux: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:40>

[Django]

#2445: [patch] allow callable values for limit_choices_to

------------------------------+------------------------------------

Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened

Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 1

------------------------------+------------------------------------
Changes (by bendavis78):

* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0

Comment:

Unsetting "needs test" and "needs documentation", as the latest patch
seems to have both.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:41>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 1
------------------------------+------------------------------------

Changes (by myusuf3):

* needs_better_patch: 0 => 1

Comment:

The newest patch no longer applies cleanly.

Setting patch needs improvement.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:42>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
------------------------------+------------------------------------

Changes (by chronos):

* cc: philipe.rp@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:43>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
------------------------------+------------------------------------

Changes (by kmike):

* cc: kmike84@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:44>

[Django]

#2445: [patch] allow callable values for limit_choices_to

------------------------------+-------------------------------------
Reporter: michael@… | Owner: zyegfryed
Type: New feature | Status: assigned

Component: Core (Other) | Version:
Severity: Normal | Resolution:

Keywords: sprint2013 | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1

------------------------------+-------------------------------------
Changes (by zyegfryed):

* keywords: => sprint2013
* owner: nobody => zyegfryed
* status: new => assigned

Comment:

I'll update the patch from @stj so it apply to master.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:46>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+-------------------------------------
Reporter: michael@… | Owner: zyegfryed
Type: New feature | Status: assigned
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 1
------------------------------+-------------------------------------

Comment (by zyegfryed):

Here's the pull-request https://github.com/django/django/pull/1228

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:47>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+-------------------------------------
Reporter: michael@… | Owner: zyegfryed
Type: New feature | Status: assigned
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
------------------------------+-------------------------------------
Changes (by jezdez):

* ui_ux: 1 => 0

Comment:

Repeating what I said on IRC, I think adding a field specific option into
the model definition is a bad idea and doesn't have precedence in the
model API. Instead I would think making it possible to pass a callable
into the ``choices`` parameter of the field as mentioned by Gary a couple
of years ago, makes more sense.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:48>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+-------------------------------------
Reporter: michael@… | Owner: zyegfryed
Type: New feature | Status: assigned
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+-------------------------------------

Comment (by jezdez):

This is related to #13181

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:49>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------

Reporter: michael@… | Owner: adamsc64

Type: New feature | Status: assigned
Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by adamsc64):

* owner: zyegfryed => adamsc64
* cc: christopher.r.adams@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:50>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner:

Type: New feature | Status: new

Component: Core (Other) | Version:
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by adamsc64):

* status: assigned => new
* owner: adamsc64 =>

Comment:

I just submitted a pull request for this ticket at
https://github.com/django/django/pull/1600.

- ForeignKey or ManyToManyField attribute limit_choices_to can now be a
callable that returns either a Q object or a dict.
- The callable will be invoked at ModelForm initialization time.
- Admin form behavior modified to handle new functionality.
- Admin widget behavior modified to handle new functionality.
- Updated Django documentation field reference section.
- Added unit tests for limit_choices_to on ModelForms.
- Tweaked unit tests for Admin to use some callables for limit_choices_to.
- I included some new tests. All old test cases run successfully.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:51>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner:
Type: New feature | Status: new

Component: Core (Other) | Version: master

Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by adamsc64):

* needs_better_patch: 1 => 0
* version: => master

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:52>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner:
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by Fak3):

* cc: someuniquename@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:53>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------

Reporter: michael@… | Owner: adamsc64
Type: New feature | Status: assigned

Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by adamsc64):

* owner: => adamsc64

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:54>

[Django]

#2445: [patch] allow callable values for limit_choices_to

-------------------------------------+-------------------------------------

Reporter: michael@… | Owner: adamsc64
Type: New feature | Status: assigned
Component: Core (Other) | Version: master
Severity: Normal | Resolution:

Keywords: sprint2013 | Triage Stage: Ready for
Has patch: 1 | checkin
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by adamsc64):

* stage: Accepted => Ready for checkin

Comment:

I submitted a new pull request that should implement this feature:
https://github.com/django/django/pull/2233

All unit tests run on my local dev box. Added new unit test
(admin_views.tests.LimitChoicesToTest) for the new feature.

Summary:

Fixed #2445 -- ``limit_choices_to`` attribute can now be a callable.

- ForeignKey or ManyToManyField attribute ``limit_choices_to`` can now
be a callable that returns either a ``Q`` object or a dict.
- The callable will be invoked at ModelForm initialization time.
- Admin form behavior modified to handle new functionality.
- Admin widget behavior modified to handle new functionality.
- Updated Django documentation field reference section.

- Added unit test class using ``limit_choices_to`` on ModelForm.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:55>

[Django]

#2445: [patch] allow callable values for limit_choices_to

------------------------------+------------------------------------
Reporter: michael@… | Owner:
Type: New feature | Status: new

Component: Core (Other) | Version: master
Severity: Normal | Resolution:

Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by adamsc64):

* status: assigned => new
* owner: adamsc64 =>
* stage: Ready for checkin => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:56>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner:
Type: New feature | Status: new
Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by timo):

* needs_better_patch: 0 => 1

Comment:

I've left some comments for improvement on the pull request.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:57>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------

Reporter: michael@… | Owner: adamsc64
Type: New feature | Status: assigned

Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by adamsc64):

* owner: => adamsc64
* status: new => assigned

Comment:

Thanks Tim. I'll make the changes as requested.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:58>

[Django]

#2445: [patch] allow callable values for limit_choices_to
------------------------------+------------------------------------
Reporter: michael@… | Owner:

Type: New feature | Status: new

Component: Core (Other) | Version: master
Severity: Normal | Resolution:
Keywords: sprint2013 | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Changes (by adamsc64):

* status: assigned => new

* needs_better_patch: 1 => 0

* owner: adamsc64 =>

Comment:

The changes requested by @timo have been made at 5d4b7a1. I updated the
pull request. See https://github.com/django/django/pull/2233.

commit 5d4b7a1c466174bfe05f32545652519446354c25
Author: Christopher Adams <christoph...@gmail.com>
Date: Sun Feb 9 12:02:03 2014 -0500

Made changes for #2445 requested by @timgraham on Feb 6, 2014.

- Moved ModelForm test of new ``limit_choices_to`` functionality to
``./tests/model_forms/*`` instead of keeping them in
``./tests/admin_views/*``.
- Added new test in admin_views for ``limit_choices_to`` callable
functionality for Django Admin.
- Updated the release notes for Django 1.7 to include a note about the
new behavior for ``limit_choices_to``.
- Added 'versionchanged' notification for ``limit_choices_to`` in the
documentation for Django 1.7.
- Added explicit link to ``raw_id_fields`` in the changes to the
``ForeignKey.limit_choices_to`` documentation.
- Defined ModelForm for tests using more canonical 'fields' instead of
'exclude'.
- Refactored datetime import statement for model_forms test.
- Phrased changes to documentation in a better way where requested.
- Added missing indentation for 'note' section of proposed changes to
documentation.
- Simplified inline code comments where requested.
- Removed unused imports.
- Refs #2445.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:59>

[Django]

#2445: [patch] allow callable values for limit_choices_to

-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted

Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Tim Graham <timograham@…>):

* owner: => Tim Graham <timograham@…>

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"eefc88feefec0c3685bfb102714530b751b4ae90"]:
{{{
#!CommitTicketReference repository=""
revision="eefc88feefec0c3685bfb102714530b751b4ae90"
Fixed #2445 -- Allowed limit_choices_to attribute to be a callable.

ForeignKey or ManyToManyField attribute ``limit_choices_to`` can now
be a callable that returns either a ``Q`` object or a dict.

Thanks michael at actrix.gen.nz for the original suggestion.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:60>

[Django]

#2445: [patch] allow callable values for limit_choices_to
-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by blueyed):

Thanks! This looks very useful.

I wonder if it could be changed to pass along any `request` object to the
callback, which would allow to look at `request.user` and then limit the
choices to objects that the current user owns?

From what I understand the `request` object may not be available, e.g.
when used via management commands, where `None` would be passed to the
callback then.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:61>

[Django]

#2445: [patch] allow callable values for limit_choices_to
-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by timo):

Not really, the `request` is generally not available in a form.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:62>

[Django]

#2445: [patch] allow callable values for limit_choices_to
-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by adamsc64):

I know some people make the `request` available in thread local storage
using middleware (see http://stackoverflow.com/questions/1057252/how-do-i
-access-the-request-object-or-any-other-variable-in-a-forms-clean-
met#answer-1057418). This effectively hacks Django to make its behavior
similar to frameworks that make the request object globally available,
such as Flask. Note however that many people believe this pattern is a bad
practice.

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:63>

[Django]

#2445: [patch] allow callable values for limit_choices_to
-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Baptiste Mispelon <bmispelon@…>):

In [changeset:"bfb11b95626f39e2f5e18d97d7761c6f93dcc1a9"]:
{{{
#!CommitTicketReference repository=""
revision="bfb11b95626f39e2f5e18d97d7761c6f93dcc1a9"
Fixed #23795 -- Fixed a regression in custom form fields

Custom form fields having a `queryset` attribute but no
`limit_choices_to` could no longer be used in ModelForms.

Refs #2445.

Thanks to artscoop for the report.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:64>

[Django]

#2445: [patch] allow callable values for limit_choices_to
-------------------------------------+-------------------------------------
Reporter: michael@… | Owner: Tim
Type: New feature | Graham <timograham@…>
Component: Core (Other) | Status: closed
Severity: Normal | Version: master
Keywords: sprint2013 | Resolution: fixed
Has patch: 1 | Triage Stage: Accepted
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Baptiste Mispelon <bmispelon@…>):

In [changeset:"606c57a132a1e1e680853c014efc41110ee75a80"]:
{{{
#!CommitTicketReference repository=""
revision="606c57a132a1e1e680853c014efc41110ee75a80"
[1.7.x] Fixed #23795 -- Fixed a regression in custom form fields

Custom form fields having a `queryset` attribute but no
`limit_choices_to` could no longer be used in ModelForms.

Refs #2445.

Thanks to artscoop for the report.

Backport of bfb11b95626f39e2f5e18d97d7761c6f93dcc1a9 from master.

Conflicts:
django/forms/fields.py
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/2445#comment:65>