contrib:admin dynamic list_display support

[Gabe Jackson]

I was trying to add dynamic list_display support for the following use-case:
The admin area of a shop should have separate views for the owner and for the supplier. the supplier should not see all the fields that the owner can see (i.e cannot see WHO ordered the products which are being requested from the supplier). For this, I wanted to have a dynamic list_display based on request.user.

We need a decent way to do this. Currently I'm using the following hacky approach which seems to work:

class MyModelAdmin(ModelAdmin):
    list_display = ('dynamicfield', 'permanent 1', 'permanent 2')
    def preprocess_list_display(self, request):
        if 'dynamicfield' not in self.list_display:
            self.list_display.insert(1, 'dynamicfield')
        if not request.user.is_superuser:
            if 'myfield' in self.list_display:
                self.list_display.remove('dynamicfield')

    def changelist_view(self, request, extra_context=None):
        self.preprocess_list_display(request)
        return super(MyModelAdmin, self).changelist_view(request)

The re-insertion of the 'dynamicfield' value is neccessary because ModelAdmin instances seem to persist. so once you log in as a non-superuser and the field is gone, the field will always be gone (even if you log back in as a superuser).

I'm not sure if it is possible, but a better appraoch would be to instanciate the ModelAdmin classes with the request, like this, one could simply overwrite init and set list_display accordingly.

[Yeago]

This could be solved without a patch using proxy models, since there may be a whole world of difference between what your two audiences see (not only in list_display, but also in the form itself).

You create a proxy model of your original model and you assign permissions to whoever based on what's appropriate.

[Alex Gaynor]

Marking as accepted, because right now the only way to solve this isn't thread safe.

[Julien Phalip]

This could be implemented in a similar way as get_readonly_fields.

[cyrus]

Implemented the get_list_display method on the ModelAdmin class. It can be used to change the list_display configuration at runtime in the same manner as get_readonly_fields.
The provided patch includes also some tests and updated documentation.

[Julien Phalip]

Thanks a lot for your work. However, you shouldn't mark your own patch as Ready For Checkin and instead get (or wait for) someone to review it.

By the way, after a very brief look at the patch, the versionadded clause should be 1.4, not 1.2.6, as the maintenance release (third-level number) is for bug fixes only and does not introduce new features. Also, version 1.3 is already out.

[Luke Plant]

The patch has various problems:

• patch needs to be against trunk, it is currently way out of date, and tests fail etc.
• tests should use RequestFactory, not another mock request object. (The existing MockRequest was almost certainly created before we had RequestFactory. Feel free to update that code with RequestFactory while you are at it).
• DynamicListDisplayChildAdmin in the tests has some code that is rather bad form - it appears to mutate the class attribute 'list_display'. In fact it doesn't, because of another implementation detail, but that shouldn't be relied on.

[cyrus]

Replying to julien:

Thanks a lot for your work. However, you shouldn't mark your own patch as Ready For Checkin and instead get (or wait for) someone to review it.

By the way, after a very brief look at the patch, the versionadded clause should be 1.4, not 1.2.6, as the maintenance release (third-level number) is for bug fixes only and does not introduce new features. Also, version 1.3 is already out.

Hey,

Thanks for the clarifications. I didn't see any progress on this issue, so I changed the stage to get some attention; and it seemed to work :) Sorry if I misunderstood the way the releases are getting out (I think I missed that part somehow). I will make the appropriate changes also based on the feedback from lukeplant and will update the patch. One question though, should I add a new patch or should I replace the existing one? Thanks!

[Julien Phalip]

Replying to cyrus:

One question though, should I add a new patch or should I replace the existing one? Thanks!

Add a new one, so that the old one can still be referred to.

[cyrus]

I updated the patch to be against trunk. Also based on the feedback from lukeplant I made the following changes:

• Updated all the tests to use RequestFactory
  
• Improved the DynamicListDisplayChildAdmin implementation to make the code more clear in terms of what it does
  

[Luke Plant]

LGTM.

[Luke Plant]

In [16340]:

Fixed #14206 - dynamic list_display support in admin

Thanks to gabejackson for the suggestion, and to cyrus for the patch.

[graveyboat]

I ran across this ticket today, but I've created some similar functionality in the past. One thing I ran across which doesn't seem to be handled here is the case where the field removed from list_display also happens to be in list_display_links. For instance, in the test, list_display = ('name','parent'). In this case, list_display_links would be [ 'name' ]. The test case covers removing 'parent', but not 'name'. If you were to remove 'name', your change_list screen would be left with a list of results with no link.

I don't want to be "that guy" that re-opens tickets, but if it needs to be re-opened, I'd be glad to submit a patch to fix this.

[Julien Phalip]

Good point. Feel free to open a new ticket with a clear description of the problem, and even a patch if possible ;)