Document ModelAdmin.lookup_allowed()

[3point2]

Right now, as a result of the security fix introduced in r15031, the only way to allow querystring lookups across relationships in the admin is to whitelist them by including them in list_filter.

However, in my application the lookup that needs to be whitelisted generates a huge filter widget as it contains thousands of instances.

It would be helpful if I could whitelist the exact lookup I need to link to without having to generate the filter widget itself.

Something like

class MyModelAdmin(ModelAdmin):
allow_lookup = ["fieldname__id__exact"]

would do. If the developers agree this is useful functionality, I could write a patch.

[Julien Phalip]

Thanks for the suggestion, but you can already easily achieve this by overriding the ModelAdmin.lookup_allowed() method. So there is no need for introducing a new class attribute.

[3point2]

Sorry to re-open. I'm fine with overriding lookup_allowed, but I opened this ticket because I feel like this is a feature that is generally useful, and lookup_allowed is undocumented. I feel like this functionality should be officially supported, and overriding an undocumented method is more of a work-around. Also see ​http://www.hoboes.com/Mimsy/hacks/fixing-django-124s-suspiciousoperation-filtering/lookup_allowed-gets-new-parameter-value/

At the very least, documenting lookup_allowed would be helpful.

If on the other hand you feel that this functionality is not a common use case, I'm fine with closing the ticket and sticking with the work around.

[Adrien Lemaire]

Renamed the ticket: Improving the documentation is a good idea.

[Julien Phalip]

I'm not sure we want to document this method yet. It has been introduced recently (in 1.2.4) to fix a security issue, and has been modified quite a bit since then, so it's quite unstable. At the very least, this needs more thought before we make it part of the official API.

[Aymeric Augustin]

Julien, do you think lookup_allowed can be considered stable now?

[Simon Meers]

Discussed in ​https://github.com/django/django/pull/8856

[Simon Meers]

See ​https://github.com/django/django/pull/8904

Though I suspect I didn't actually need to create three separate pull requests for the three (dependent but distinct) commits?

[Tim Graham]

I suppose your thinking was that since ModelAdmin.lookup_allowed() isn't documented, it's okay to make a backwards-incompatible change in adding the request argument without a deprecation? Lately, Django has been more conservative about changing undocumented APIs without a deprecation if there's evidence that the API is at least somewhat widely used by projects. A ​GitHub code search reveals that's probably the case here.

If I were writing a patch, I would first document ModelAdmin.lookup_allowed() as it is now and backport that documentation to 1.11. (A separate pull request would ease merging that incrementally). Then I'd implement a deprecation and corresponding documentation changes for adding the request argument as part of #22569.

[Simon Meers]

Thanks Tim, sounds like a good approach. Here is a documentation patch for the current implementation to start with: ​https://github.com/django/django/pull/8906

[Simon Meers]

So the deprecation would involve:

• Adding a heads-up in internals/deprecation.txt that request will be required in 2.0 (or do we have to go further ahead -- 2.1?)
• Adding code to lookup_allowed in the current implementation that accepts both parameter formats (with/without request) and provides a deprecation warning if request is absent -- can this be in 1.11, or does that need to be in 2.0?
• Then performing the change in ​https://github.com/django/django/pull/8904 (and removing the deprecation warning)
• Hopefully ​https://github.com/django/django/pull/8903 can be merged in the meanwhile to close #28496 and lay the foundation for the simpler patch for #22569?

[Tim Graham]

The deprecation should start in Django 2.0 and request would be required in Django 3.0 (use RemovedInDjango30Warning). You may find the checklist for ​Deprecating a feature helpful.

I plan to look at the patch for #28496 tomorrow.

[Tim Graham <timograham@…>]

In 60443e84:

Fixed #17985 -- Documented ModelAdmin.lookup_allowed().

[Tim Graham <timograham@…>]

In 07f73da:

[1.11.x] Fixed #17985 -- Documented ModelAdmin.lookup_allowed().

Backport of 60443e84b38ea3a143b0ef9c05b1e1f39d91ddb5 from master