staff_member_required feels like a bit of a relic from the past:

- It looks like before a massive refactor of the admin that happened 12
years ago (
https://github.com/django/django/commit/a19ed8aea395e8e07164ff7d85bd7dff2f24edca)
staff_member_required played a similar role to what Adminsite.admin_view
does now (checks permissions and redirects to login if appropriate). Before
the refactor, all of the relevant views were wrapped with
staff_member_required.
- The above commit introduced AdminSite which has its own 'has_permissions'
method. Adminsite.root (which was the old get_urls) called
self.has_permissions before calling any other views, so
staff_member_required was now no longer really required.
- staff_member_required did however continue to be used in the admindocs
app (which was now split off into its own app).
- Adminsite.root was eventually replaced with get_urls and the call to
has_permissions  was moved into `Adminsite.admin_view` (which also does a
few other things: adds csrf protection, and deals with caching)
- Over time staff_member_required became simpler and simpler as things like
user_passes_test were introduced, and there was no need to repeat logic.
- It's now just a very specific version of `user_passes_test` and is only
used internally one place - the admindocs app.


Tobias - In terms of permissions and redirecting; the behaviour of
admin_view and `staff_member_required` are very similar. The differences
are that:
 i) admin_view defers to Adminsite.has_permission to do the actual checking
of permissions (so if that method is overwritten the behaviour will change)
 ii) admin_view does a whole load of other things that all admin views are
going to need (mentioned above).
 iii) They have slightly different arguments you can pass in to modify the
behaviour

If you are decorating a view of ModelAdmin, or AdminSite you should use
admin_view - as described in the docs under get_urls.

Since staff_member_required is more or less a very specific version of
user_passes_test designed for admin views, it's use-case seems pretty
niche. If you were making an app like admindocs which behaves like an
extension of the admin app, without actually being part of the admin app
you could use it then, but I'm struggling to think of any other time you
would want to use it though.

I think there's an argument to be made for deprecating it completely:
 - It's only used in one place internally, and the wider use case seems
pretty niche.
 - user_passes_test can do the same thing and seems to be the standard way
of doing this kind of thing anyway.
 - user_passes_test with a suitably named test argument would also be more
explicit, since the login_url and redirect_field_name would need to be
explicitly stated.

 Definitely the documentation should be updated though. Thoughts on
deprecating it?

Tim

On Tue, Mar 9, 2021 at 7:08 PM Tobias Bengfort <tobias.bengf...@posteo.de>
wrote:

> On 09/03/2021 18.03, Carlton Gibson wrote:
> > Do you want to open a PR with suggested changes as a focus for
> discussion?
>
> I would open a PR that improves the docs, but I still don't understand
> the difference, so I wouldn't know what to write.
>
> The other alternative would be remove one of them, but at this point
> that is probably too drastic.
>
> tobias
>
> --
> You received this message because you are subscribed to the Google Groups
> "Django developers  (Contributions to Django itself)" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to django-developers+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/django-developers/d10799d4-8a42-6caa-8e13-c2ce93256521%40posteo.de
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/CAFBPPP0bM2cr9etcMQwmprK1zX8gfKZ2wmEa7BW2CyiCrxTUog%40mail.gmail.com.
  • Dif... Tobias Bengfort
    • ... Carlton Gibson
      • ... 'Adam Johnson' via Django developers (Contributions to Django itself)
        • ... Tobias Bengfort
          • ... Carlton Gibson
            • ... Tobias Bengfort
              • ... Carlton Gibson
                • ... Tobias Bengfort
                • ... Timothy McCurrach
                • ... Matthew Pava
                • ... Timothy McCurrach
                • ... Ian Foote
                • ... Kevin Grinberg
                • ... 'Adam Johnson' via Django developers (Contributions to Django itself)

Reply via email to