contrib:admin dynamic list_display_links support

[graveyboat]

Ticket #14206 created a very useful way to allow programmers to override list_display at runtime by using a new function get_list_display instead of accessing self.list_display. Unfortunately, it potentially causes an issue with list_display_links.

If list_display_links is not set in the ModelAdmin instance, currently the first field after 'action_checkbox' in list_display becomes list_display_links. Since get_list_display happens after list_display_links is set, there is a potential that what is returned from get_list_display no longer contains the field in list_display_links. Take the following for example:

class DynamicListDisplayChildAdmin(admin.ModelAdmin):
    list_display = ('name', 'parent')

    def get_list_display(self, request):
        my_list_display = list(self.list_display)
        if request.user.username == 'noparents':
            my_list_display.remove('name')

        return my_list_display

In this case, list_display_links would be name, but 'name' is no longer in the list_display passed to the ChangeList instance. Basically, the fix is to create a get_list_display_links method that is capable of checking list_display if necessary.

[graveyboat]

Patch for the proposed change

[graveyboat]

Implemented the change with this patch, as well as provided a test and documentation.

[Aymeric Augustin]

This looks useful.

In the patch, it would be more readable to build list_display_links = list(self.get_list_display_links(request, list_display)) outside the invocation of ChangeList(...), just after building list_display.

[graveyboat]

Updated the patch to build list_diplay_links outside the invocation of ChangeList

[Julien Phalip]

Definitely a good idea, however the suggested approach could be made more robust. In particular, it's a shame having to override get_list_display_links() nearly every time get_list_display() is overridden to avoid any missing field. I've posted a new patch which moves the initialisation of the list display fields and links right into the hooks, which means that by default get_list_display_links() will always pick the first field returned by get_list_display().

This would introduce a slight backwards incompatible change as the initialisation would be done at runtime instead of when ModelAdmin itself is initialised. I haven't had enough time to think about whether this would be an issue that we need to care about -- so some feedback would be welcome.

The test could also be improved to make the result of get_list_display_links() vary based on the request and list_display parameters.

[Julien Phalip]

While working on this I've realised that there were some issues with get_list_display() which I think should be addressed beforehand. Those issues are described in #17090.

[Julien Phalip]

In [17037]:

Fixed #16257 -- Added new ModelAdmin.get_list_display_links() method to allow for the dynamic display of links on the admin changelist. Thanks to graveyboat for the suggestion and initial patch.