Re: The blacklist / master issue

2021-03-09 Thread Markus Holtermann
Hi all,

Mariusz renamed the branches this morning and merged the corresponding pull 
requests. Thank you!

Please let us know if you spot problems so they can be fixed.

Cheers,

Markus

On Tue, Mar 2, 2021, at 6:05 PM, Markus Holtermann wrote:
> Brief update on this.
> 
> The overall tracking pull request ist 
> https://github.com/django/django/pull/14048/
> 
> * On 2021-03-03 at UTC morning, well migrate 
> django/code.djangoproject.com and django/djangoproject.com
> 
> * Likely on 2021-03-09 we'll migrate django/django
> 
> Cheers,
> 
> Markus
> 
> On Thu, Feb 25, 2021, at 7:31 PM, Markus Holtermann wrote:
> > Thanks for the input, Matthias. That's useful to know. I'll make sure 
> > the change is announced.
> > 
> > Cheers,
> > 
> > Markus
> > 
> > On Thu, Feb 25, 2021, at 7:24 PM, Matthias Kestenholz wrote:
> > > Yes, please.
> > > 
> > > As a third party app developer I'll have to update a few GitHub 
> > > workflows and tox configurations (since I'm running testsuites against 
> > > the main branch too). It may be useful to announce this change on as 
> > > many channels as possible (mailing lists, the forum, as a news entry on 
> > > the Django website). Just an idea, this shouldn't be a blocker for 
> > > moving forward with this.
> > > 
> > > Thanks,
> > > Matthias
> > > 
> > > 
> > > On 24/02/2021 00:12, 'Adam Johnson' via Django developers (Contributions 
> > > to Django itself) wrote:
> > > > Yes, let's do it. I did it to my open source projects a couple weeks 
> > > > ago 
> > > > and everything has been smooth since. We'll need some find/replace for 
> > > > links in the main repo, on djangoproject.com 
> > > > , 
> > > > and I imagine some other places.
> > > > 
> > > > On Tue, 23 Feb 2021 at 22:15, Kenneth  > > > > wrote:
> > > > 
> > > > I agree. We should go ahead and do the switch
> > > > 
> > > > On Tue, Feb 23, 2021 at 11:57 AM Markus Holtermann
> > > > mailto:i...@markusholtermann.eu>> wrote:
> > > > 
> > > > Hi all,
> > > > 
> > > > Reviving an old topic. GitHub has by now tooling in place to
> > > > rename branches and keep open PRs in sync. In fact, if I were to
> > > > change the `master` branch to `main`, GitHub tells me this:
> > > > 
> > > > Renaming this branch:
> > > > * Will update 158 pull requests targeting this branch across 112
> > > > repositories.
> > > > * Will update 1 branch protection rule that explicitly targets
> > > > master.
> > > > * Will not update your members' local environments.
> > > > 
> > > > I'd suggest to go through with this change and make the
> > > > necessary changes to the CI / website.
> > > > 
> > > > Cheers,
> > > > 
> > > > Markus
> > > > 
> > > > 
> > > > On Tue, Jun 23, 2020, at 11:04 AM, Mark Bailey wrote:
> > > >  > +1 on a good decision to make this change.
> > > >  >
> > > 
> > > -- 
> > > 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/81446dcd-e04c-3c28-91b5-a276a38baaf7%40feinheit.ch.
> > >
> > 
> > -- 
> > 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/8caebe33-6b52-46f7-8f7b-b324474a546b%40www.fastmail.com.
> >
> 
> -- 
> 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/dd0f1e1a-3e6b-4202-8c55-f0741b35f88e%40www.fastmail.com.
>

-- 
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/6eadf150-3208-4300-bedb-c2b615e023f2%40www.fastmail.com.


Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Tobias Bengfort

Hi,

while reading the documentation on django's admin site I noticed there 
are two mechanisms to check for authentication: AdminSite.admin_view[0] 
and staff_member_required[1]. I am not sure when I should use which of them.


So what is the difference between the two? I expect that 
staff_member_required is more low-level and I should use 
AdminSite.admin_view in most cases. Is that correct?


I will try to make a pull request to add the answer to the documentation.

thanks
tobias


[0]: 
https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_urls
[1]: 
https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator


--
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/cb52dfda-67e4-a336-4eea-39195d6f8bb3%40posteo.de.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Carlton Gibson
Hi Tobias. 

I’ve always taken it that @staff_member_required is for decorating views that 
aren’t part of the admin, and so not accessed via AdminSite.get_urls(). 

I don’t think I’d use AdminSite.admin_view() outside of the context in the 
example there. 
I can’t think why one would do that… 🤔 — no doubt someone does 🙂

Kind Regards,

Carlton


> On 9 Mar 2021, at 11:56, Tobias Bengfort  wrote:
> 
> Hi,
> 
> while reading the documentation on django's admin site I noticed there are 
> two mechanisms to check for authentication: AdminSite.admin_view[0] and 
> staff_member_required[1]. I am not sure when I should use which of them.
> 
> So what is the difference between the two? I expect that 
> staff_member_required is more low-level and I should use AdminSite.admin_view 
> in most cases. Is that correct?
> 
> I will try to make a pull request to add the answer to the documentation.
> 
> thanks
> tobias
> 
> 
> [0]: 
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_urls
> [1]: 
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator
> 
> -- 
> 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/cb52dfda-67e4-a336-4eea-39195d6f8bb3%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/67A3688D-41C0-4E14-B235-51482689D285%40gmail.com.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
admin_view does extra stuff like calling the admin site's get_permission
method and using the admin login page rather than the default auth one:
https://github.com/django/django/blob/98d3fd61026457a435ef5b7afce6b6e64e9f241d/django/contrib/admin/sites.py#L198

It should indeed be used only for admin pages.

On Tue, 9 Mar 2021 at 11:04, Carlton Gibson 
wrote:

> Hi Tobias.
>
> I’ve always taken it that @staff_member_required is for decorating views
> that aren’t part of the admin, and so not accessed via
> AdminSite.get_urls().
>
> I don’t think I’d use AdminSite.admin_view() outside of the context in the
> example there.
> I can’t think why one would do that… 🤔 — no doubt someone does 🙂
>
> Kind Regards,
>
> Carlton
>
>
> > On 9 Mar 2021, at 11:56, Tobias Bengfort 
> wrote:
> >
> > Hi,
> >
> > while reading the documentation on django's admin site I noticed there
> are two mechanisms to check for authentication: AdminSite.admin_view[0] and
> staff_member_required[1]. I am not sure when I should use which of them.
> >
> > So what is the difference between the two? I expect that
> staff_member_required is more low-level and I should use
> AdminSite.admin_view in most cases. Is that correct?
> >
> > I will try to make a pull request to add the answer to the documentation.
> >
> > thanks
> > tobias
> >
> >
> > [0]:
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#django.contrib.admin.ModelAdmin.get_urls
> > [1]:
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator
> >
> > --
> > 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/cb52dfda-67e4-a336-4eea-39195d6f8bb3%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/67A3688D-41C0-4E14-B235-51482689D285%40gmail.com
> .
>


-- 
Adam

-- 
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/CAMyDDM0DraaK372CuuwueqJqBieiTvmQbkwNVH8ABBr1ksURRA%40mail.gmail.com.


Re: 'npm init django' works, maybe let's stop that?

2021-03-09 Thread 'Adam Johnson' via Django developers (Contributions to Django itself)
I re-submitted my proposal through the "contact the foundation" form (
https://www.djangoproject.com/contact/foundation/ ) since that is the only
way I know of contacting the board.

On Mon, 8 Mar 2021 at 08:09, Florian Apolloner 
wrote:

> Mhm, I expected such an answer when I hit send :) I maybe should have
> worded it better. While you are right, that ultimately this is an issue for
> the DSF board I think that most of those issues can be solved by asking
> authors/etc nicely without having to invoke the thread of trademarks or
> lawyers. When that fails then we can surely put it forward to the DSF.
>
> On Monday, March 8, 2021 at 9:04:49 AM UTC+1 James Bennett wrote:
>
>> It's not really a discussion of whether "we" (this list) want
>> something removed; how to enforce Django's trademark is purely the
>> domain of the DSF. The DSF also has the ability to wave its ownership
>> of the trademark around and hire lawyers in cases where people are
>> reluctant to take something down, which has happened in the past. So
>> again I'd say that if people are putting packages up that seem to
>> conflict with the Django trademark, just notify the DSF board and let
>> them handle it.
>>
> --
> 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/dbbb7440-8eea-4d4a-9f3f-85aba577edb1n%40googlegroups.com
> 
> .
>


-- 
Adam

-- 
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/CAMyDDM14Y%3DkHZrr-0weDurp9e4mXpQy82GJppY2g3qtOJKGFiA%40mail.gmail.com.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Tobias Bengfort

Hi,

On Tue, 9 Mar 2021 at 11:04, Carlton Gibson wrote:
> I’ve always taken it that @staff_member_required is for decorating
> views that aren’t part of the admin, and so not accessed via
> AdminSite.get_urls().

On 09/03/2021 13.21, Adam Johnson wrote:
admin_view does extra stuff like calling the admin site's get_permission 
method and using the admin login page rather than the default auth one: 


According to [0] staff_member_required also uses the admin login page by 
default. That contradicts both your answers, doesn't it? Now I am more 
confused than before.


tobias

[0] 
https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator


--
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/c1ff3386-e50d-9de6-816b-8019b5d25e73%40posteo.de.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Carlton Gibson
Hi.

> On 9 Mar 2021, at 14:33, Tobias Bengfort  wrote:
> 
> According to [0] ...
> 
> [0] 
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator

Those docs are unchanged since they were added 6 years ago. 
https://github.com/django/django/commit/e8a758e941bb36f69b6224b73b00b9d6a814bbdc
 


A PR modernising the explanation may be in order. (I’d need to look at the 
matching history for admin_view judge fully…)

Kind Regards,

Carlton

-- 
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/12592C48-40D3-40FF-832E-D6D14DA8B9B4%40gmail.com.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Tobias Bengfort

I checked the git logs and learned about a neat git feature:

git log -L ':staff_member_required:django/contrib/admin/views/decorators.py
git log -L ':admin_view:django/contrib/admin/sites.py

(requires `*py diff=python` in .gitattributes)

The last commit to admin_view was in 2009. The last commit to 
staff_member_required was 2015. It was documented shortly after that.


So it seems like the documentation is up to date with the code.

On 09/03/2021 14.50, Carlton Gibson wrote:

Hi.

On 9 Mar 2021, at 14:33, Tobias Bengfort > wrote:


According to [0] ...

[0] 
https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator 



Those docs are unchanged since they were added 6 years ago.
https://github.com/django/django/commit/e8a758e941bb36f69b6224b73b00b9d6a814bbdc 



A PR modernising the explanation may be in order. (I’d need to look at 
the matching history for admin_view judge fully…)


Kind Regards,

Carlton

--
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/12592C48-40D3-40FF-832E-D6D14DA8B9B4%40gmail.com 
.


--
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/5df94dc8-34f2-5484-15a9-bddac2eaec52%40posteo.de.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Carlton Gibson
Hey Tobias.

OK, it looks like I need to review it in more depth. Possibly it does all
make sense, but the staff_member_required docs don’t read how I’d expect
them to from this conversation.

Do you want to open a PR with suggested changes as a focus for discussion?

Kind regards,
Carlton

On Tue, 9 Mar 2021 at 17:15, Tobias Bengfort 
wrote:

> I checked the git logs and learned about a neat git feature:
>
> git log -L ':staff_member_required:django/contrib/admin/views/decorators.py
> git log -L ':admin_view:django/contrib/admin/sites.py
>
> (requires `*py diff=python` in .gitattributes)
>
> The last commit to admin_view was in 2009. The last commit to
> staff_member_required was 2015. It was documented shortly after that.
>
> So it seems like the documentation is up to date with the code.
>
> On 09/03/2021 14.50, Carlton Gibson wrote:
> > Hi.
> >
> >> On 9 Mar 2021, at 14:33, Tobias Bengfort  >> > wrote:
> >>
> >> According to [0] ...
> >>
> >> [0]
> >>
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator
> >> <
> https://docs.djangoproject.com/en/3.1/ref/contrib/admin/#the-staff-member-required-decorator
> >
> >
> > Those docs are unchanged since they were added 6 years ago.
> >
> https://github.com/django/django/commit/e8a758e941bb36f69b6224b73b00b9d6a814bbdc
> > <
> https://github.com/django/django/commit/e8a758e941bb36f69b6224b73b00b9d6a814bbdc
> >
> >
> > A PR modernising the explanation may be in order. (I’d need to look at
> > the matching history for admin_view judge fully…)
> >
> > Kind Regards,
> >
> > Carlton
> >
> > --
> > 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/12592C48-40D3-40FF-832E-D6D14DA8B9B4%40gmail.com
> > <
> https://groups.google.com/d/msgid/django-developers/12592C48-40D3-40FF-832E-D6D14DA8B9B4%40gmail.com?utm_medium=email&utm_source=footer
> >.
>
> --
> 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/5df94dc8-34f2-5484-15a9-bddac2eaec52%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/CAJwKpyQtVVsh%3DH1zv-JhLETNUH%3D%2B64w%3DZquAEGOOO4HDhTQiyw%40mail.gmail.com.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Tobias Bengfort

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.


Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Timothy McCurrach
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 
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.


RE: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Matthew Pava
> Thoughts on deprecating it?
It seems like a good idea. However, it leads us to another question. Do we 
really need the is_staff field on the User Model if it is deprecated?

From: django-developers@googlegroups.com  
On Behalf Of Timothy McCurrach
Sent: Tuesday, March 9, 2021 3:22 PM
To: django-developers@googlegroups.com
Subject: Re: Difference between AdminSite.admin_view and staff_member_required?

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 
mailto: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 rece

Re: Difference between AdminSite.admin_view and staff_member_required?

2021-03-09 Thread Timothy McCurrach
> It seems like a good idea. However, it leads us to another question. Do
we really need the is_staff field on the User Model if it is deprecated?

User.is_staff is still used, just from Adminsite.has_permission instead:
https://github.com/django/django/blob/main/django/contrib/admin/sites.py#L196

On Tue, Mar 9, 2021 at 9:45 PM Matthew Pava  wrote:

> > Thoughts on deprecating it?
>
> It seems like a good idea. However, it leads us to another question. Do we
> really need the is_staff field on the User Model if it is deprecated?
>
>
>
> *From:* django-developers@googlegroups.com <
> django-developers@googlegroups.com> *On Behalf Of *Timothy McCurrach
> *Sent:* Tuesday, March 9, 2021 3:22 PM
> *To:* django-developers@googlegroups.com
> *Subject:* Re: Difference between AdminSite.admin_view and
> staff_member_required?
>
>
>
> 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 
> 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
>