Pass "obj" parameter to get_inline_instances

[ybon]

In the 1.4 rc1 release, a get_inline_instances method has been added in [source:/django/trunk/django/contrib/admin/options.py#L348 contrib.admin].

This method does not accept the "obj" parameter.

This is not consistent with the other methods of ModelAdmin:

• get_formset
• get_form
• get_fieldsets
• get_prepopulated_fields
• get_readonly_fields

My opinion is that the faster we make this change, the less painful it will be in the future.

A patch is provided.

(Discussion in ​this thread.)

[ybon]

Adding "obj" parameter to get_inline_instances method in contrib.admin

[Aymeric Augustin]

First, get_inline_instances wasn't introduced as a new API, but to fix a permission-checking bug on inlines. See r16934. Viewing it a a hook for admin customization is interesting :)

Currently, it isn't documented, which makes it an internal API. And we don't commit to keeping backwards compatibility for internal APIs. If you use or override them and they change in a later version of Django, you're on your own -- and in this case it will be trivial to fix. That means we could make that change after 1.4.

Alternatively, if we decide it's a public of "semi-public" API, and try not to break backwards compatibility, we need the API to be correct and consistent with surrounding code before it's set in stone. If we're choosing this path, this bug is a release blocker.

I'm marking this as a release blocker because we must decide between (a) accepting to change the API after 1.4 (b) fix it now. If we choose (a) then it's a normal ticket, if we choose (b) then it's a release blocker.

Given this situation, the patch being as harmless as they come, and the convincing (IMO) arguments of Yohan on the mailing list, I'm seriously tempted to make the change.

[Julien Phalip]

It's a shame that this feature is missing, however it is too late to be considered for 1.4. The admin doesn't really have a strict backwards-compatibility policy, so we can consider fixing this in 1.5.

[domguard]

What can be done to see this happen ?

[Aymeric Augustin]

The idea is accepted, but the current patch lacks tests and docs. You can add them and upload a new patch.

[domguard]

@aaugustin : Ok, I'm looking for examples of regression tests on the other related methods mentioned in the ticket, I can't find any...
Apart of passing another object, that will be ignored by past code, this code is quite simple (and works)
Please aaugustin can you just give me an idea of what tests could be made here ?

[domguard]

Updated and tested patch (the add_view case had been omitted)
The patch is really simple : it adds the obj parameter to the method, and passes it in the three only places where the get_inline_instances is called. And this makes it behave just like its cousins ModelAdmin methods.

More can be said about the need of an entry point here.
For example, this is the only clean way I found (​https://groups.google.com/forum/?fromgroups#!topic/django-users/l_nsr0_ea0o) to pass the parent object instances to inlines (Inline classes have to be modified to get the arg, but this is another possible enhancement once this one exists)
You need to know which object you're working on when you need to filter an inline form field queryset based on the parent object, a rather common pattern...

[Aymeric Augustin]

#18619 was a duplicate with a pull request: ​https://github.com/django/django/pull/202

[Stephan Jaensch]

As the author of the pull request (and the one that's to blame for the omission in the definition of get_inline_instances()) I can verify that the patch in this bug fixes the issue without introducing unwanted side effects. In fact it's almost identical to my independently written pull request. The method still works when called without the obj parameter and behaves exactly the same as in Django 1.4.

[Stephan Jaensch]

On second thought: I do actually like my pull request a bit better since it passes the obj parameter along to the permission checking functions instead of just ignoring it.

[domguard]

Last version of the pull request ​https://github.com/django/django/pull/202 supersedes the above patch , do we need to create a new patch here, or does github pull requests are now accepted in the django project workflow ?

[Aymeric Augustin]

Pull requests are a accepted as well as patches.

[Stephan Jaensch]

Hey quinode, could you (or someone else) review this small patch and mark the ticket as "Ready for checkin" if applicable? It would be really nice to fix this oversight for 1.5.

[domguard]

Hey @sjaensch I just pushed a documentation update to complete your patch branch
Please merge it and mark this as "Ready for checkin", we'll see what happens :)

[Stephan Jaensch]

Done.

[Aymeric Augustin <aymeric.augustin@…>]

In c2e19e26bc33d34eff57079bd1a6838ff64d9e81:

Fixed #17856 -- Passed obj to get_inline_instances

Thanks ybon, quinode and sjaensch for the patch, and Tim Graham
for the review.