87 views
Skip to first unread message

Rick van Hattem

unread,
Jun 15, 2015, 3:52:36 PM6/15/15
to django-d...@googlegroups.com
On 15 June 2015 at 21:34, Florian Apolloner <f.apo...@gmail.com> wrote:


On Monday, June 15, 2015 at 7:07:38 PM UTC+2, Rick van Hattem (wolph) wrote:
Would anyone oppose a pull request like this?

Yes, it is highly backwards incompatible for not much gain, I am also usually just fine with one/two fields for list_display. You could just use your own admin subclass for that.

Can you clarify on that? I don't see the backwards incompatibility here.

Upgrading or downgrading Django won't break this in any way and it won't change the way people can override the list_display default value so by definition it's not breaking either forwards or backwards compatibility. It changes behaviour and looks, but it doesn't break anything.

The discussion here shouldn't be whether you can or cannot fix it yourself (obviously, you can, that's not the issue), it's what a good/sane default would be. For brand new Django users, would it be more convenient to have 1 column or just all local columns and make it slightly more usable?

~wolph

Shai Berger

unread,
Jun 15, 2015, 5:03:07 PM6/15/15
to django-d...@googlegroups.com
(I received the message I'm replying to here with an empty subject, and
detached from the thread. Google Groups being funny?)

On Monday 15 June 2015 22:52:09 Rick van Hattem wrote:
> On 15 June 2015 at 21:34, Florian Apolloner <f.apo...@gmail.com> wrote:
> > On Monday, June 15, 2015 at 7:07:38 PM UTC+2, Rick van Hattem (wolph)
> >
> > wrote:
> >> Would anyone oppose a pull request like this?
> >
> > Yes, it is highly backwards incompatible for not much gain, I am also
> > usually just fine with one/two fields for list_display. You could just
> > use your own admin subclass for that.
>
> Can you clarify on that? I don't see the backwards incompatibility here.
>

It could quite easily cause breakage for specific client-side code, although I
wouldn't consider that "highly" incompatible.

However, it could also easily lead to inappropriate data exposure -- where
people who are supposed to get an "opaque" view of some objects will, upon
upgrade, be able to see all their details. I consider that risk to weigh much
more than the potential gains.
>
> The discussion here shouldn't be whether you can or cannot fix it yourself
> (obviously, you can, that's not the issue), it's what a good/sane default
> would be. For brand new Django users, would it be more convenient to have 1
> column or just all local columns and make it slightly more usable?
>
Beside "convenient", you should also consider "safe", and besides brand new
users, there are also established users with significant codebases. Now,
arguably, if we were starting the Django project today, we could use the
default you propose, people would be aware of it, and if they wanted to limit
access, they would. One could still argue that "whitelisting" is better than
"blacklisting", and we could have a whole discussion about this. But having a
Django upgrade just expose more data by default, even in the Admin, would be a
serious breach of our users' trust IMO.

Shai.

Marc Tamlyn

unread,
Jun 15, 2015, 5:59:51 PM6/15/15
to django-d...@googlegroups.com

Agree this is not an appropriate default, although I could see an argument for supporting __all__ like in forms. This isn't very hard as a third party solution though so I'm not super keen on that idea.

M

Rick van Hattem (wolph)

unread,
Jun 22, 2015, 8:48:27 AM6/22/15
to django-d...@googlegroups.com
On Monday, June 15, 2015 at 11:03:07 PM UTC+2, Shai Berger wrote:
(I received the message I'm replying to here with an empty subject, and
detached from the thread. Google Groups being funny?)

Guess so, I didn't get an email from your reply either... that's why I responded this slowly.

On Monday 15 June 2015 22:52:09 Rick van Hattem wrote:
> On 15 June 2015 at 21:34, Florian Apolloner <f.apo...@gmail.com> wrote:
> > On Monday, June 15, 2015 at 7:07:38 PM UTC+2, Rick van Hattem (wolph)
> >
> > wrote:
> >> Would anyone oppose a pull request like this?
> >
> > Yes, it is highly backwards incompatible for not much gain, I am also
> > usually just fine with one/two fields for list_display. You could just
> > use your own admin subclass for that.
>
> Can you clarify on that? I don't see the backwards incompatibility here.
>

It could quite easily cause breakage for specific client-side code, although I
wouldn't consider that "highly" incompatible.

However, it could also easily lead to inappropriate data exposure -- where
people who are supposed to get an "opaque" view of some objects will, upon
upgrade, be able to see all their details. I consider that risk to weigh much
more than the potential gains.

Ah, yes... you are right, I didn't take that into consideration. That is a very valid argument, you have persuaded me that it shouldn't change the behaviour in all cases.

 
>
> The discussion here shouldn't be whether you can or cannot fix it yourself
> (obviously, you can, that's not the issue), it's what a good/sane default
> would be. For brand new Django users, would it be more convenient to have 1
> column or just all local columns and make it slightly more usable?
>
Beside "convenient", you should also consider "safe", and besides brand new
users, there are also established users with significant codebases. Now,
arguably, if we were starting the Django project today, we could use the
default you propose, people would be aware of it, and if they wanted to limit
access, they would. One could still argue that "whitelisting" is better than
"blacklisting", and we could have a whole discussion about this. But having a
Django upgrade just expose more data by default, even in the Admin, would be a
serious breach of our users' trust IMO.

Generally I'm a great proponent of whitelisting versus blacklisting, with the admin, I'm not because by default all of the fields are already visible when editing. So right now it's just inconsistent between the two.

Although I'm not a fan of magic, if the fields and/or fieldset are not set within the admin, they're already visible so in that case it might still be a good default. Conditional defaults are less than optimal of course... what are your thoughts about a solution like that?

Or as Marc suggests, supporting __all__ would help a lot too.
Reply all
Reply to author
Forward
0 new messages