Opened 5 years ago

Last modified 6 weeks ago

#13659 new New feature

Make the request accessible in callables used in ModelAdmin.list_display

Reported by: sebastian_noack Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: ionel.mc@…, django@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Its a nice feature of the dajngo admin that you can use names of model or model admin methods and plain callables in ModelAdmin.list_display. But I want show content in my custom column dependant on the logged in user. This isn't possible at the moment. But I have written a patch that passes the request object to the given callable or method, when it has the needs_request attribute and it is True.

Attachments (1)

pass_request_to_admin_display_callbacks.patch (8.0 KB) - added by sebastian_noack 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 years ago by sebastian_noack

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by jacob

  • Needs documentation set
  • Needs tests set
  • Patch needs improvement set
  • Triage Stage changed from Unreviewed to Accepted

Nitpick: I'd name the function attribute accepts_request and not needs_request -- reads better to me. Other than that looks good, but needs docs and tests.

comment:3 Changed 5 years ago by sebastian_noack

I agree, the name sounds better, indeed.

comment:4 Changed 5 years ago by sebastian_noack

  • Needs tests unset
  • Patch needs improvement unset

I have updated the patch to django 1.2, renamed needs_request and have added a test, now.

Changed 5 years ago by sebastian_noack

comment:5 Changed 5 years ago by gabejackson

i think it would be better pass request in to the constructor of ModelAdmin and create an additional method "get_list_display" so one can dynamically set the list_display fields dependent of the request (i.e have a different list_display depending on request.user or similar). It might be even better to register ModelAdmin objects dynamically based on request instead of doing all this conditional get_* overrides in ModelAdmin.
Another question would be if it's possible to limit access of users to certain AdminSites. this would be another solution:
have multiple AdminSites and register the appropriate ModelAdmins. Currently I think that users may log in to any AdminSite though, or am I mistaking?

comment:6 Changed 5 years ago by anonymous

  • Cc ionel.mc@… added

comment:7 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to New feature

comment:8 Changed 4 years ago by julien

  • Easy pickings unset
  • Patch needs improvement set

I think there's a problem with this patch. The request parameter in lookup_field could be None, which would happen when called from AdminReadonlyField.contents(). This means that the callable may receive None in some cases, which is a shame. The callable should always receive the HTTPRequest object.

Also, I think it would make more sense (and be more consistent with the way ModelAdmin's methods work) if the request was provided as the first argument, e.g.:

    def my_function(request, ...):
        ...

or:

    def my_method(self, request, ...):
        ...

And finally, I'd prefer the function attribute to be called takes_request, for consistency with takes_context in the inclusion_tag.

comment:9 Changed 4 years ago by jedie

  • Cc django@… added
  • UI/UX unset

comment:10 Changed 6 weeks ago by timgraham

  • Summary changed from Made the request accessible in callables used in ModelAdmin.list_display [PATCH] to Make the request accessible in callables used in ModelAdmin.list_display
  • Version changed from 1.2 to master
Note: See TracTickets for help on using tickets.
Back to Top