Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #22569 -- Allowed ModelAdmin.lookup_allowed() to respect get_list_filter(). #8856

Closed
wants to merge 2 commits into from

Conversation

DrMeers
Copy link
Member

@DrMeers DrMeers commented Aug 7, 2017

@DrMeers DrMeers force-pushed the 22569_lookup_allowed branch 2 times, most recently from 915459a to 24f491b Compare August 7, 2017 04:09
@DrMeers
Copy link
Member Author

DrMeers commented Aug 7, 2017

Actually I wonder whether we should just make the request parameter mandatory since it isn't fully backwards-compatible if you've written a custom lookup_allowed method that doesn't accept request.

Copy link
Member

@timgraham timgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would first document lookup_allowed() (https://code.djangoproject.com/ticket/17985) and then implement a deprecation path for adding the request argument, similar to 487362f, rather than making the backwards incompatible change for existing overrides of lookup_allowed().

* ``ModelAdmin.lookup_allowed`` now accepts a ``request`` parameter to ensure
it respects ``get_list_filter``. The parameter is optional to maintain
backwards compatibility.
* A new ``ModelAdmin.get_changelist_instance`` method creates new override
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than using this in the new test, I don't see how it's linked to the rest of the PR. I think it should have its own ticket, tests, docs, and PR. That'll help simplify this one.

@DrMeers
Copy link
Member Author

DrMeers commented Aug 8, 2017

Agreed on both counts, thanks @timgraham

@timgraham timgraham changed the title Fixed #22569 -- lookup_allowed now accepts request to respect get_list_filter Fixed #22569 -- Allowed ModelAdmin.lookup_allowed() to respect get_list_filter(). Sep 2, 2017
@timgraham
Copy link
Member

Simon, do you want to continue this?

@timgraham
Copy link
Member

Closing due to inactivity.

@timgraham timgraham closed this Jun 29, 2018
@DrMeers
Copy link
Member Author

DrMeers commented Jun 30, 2018

Sorry @timgraham , I really do want to continue this but am not sure if/when I'll find the time :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants