Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by timgraham):

 If you could write a patch, I'll be happy to review it.

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.2bd24d2ef2ef48f4dbd810c6f6f06e82%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by synotna):

 This bug just hit me: I had set my base fields/readonly_fields on the
 ModelAdmin, and overwrote get_fields & get_readonly_fields thinking it
 would /always/ modify the base fields

 I highly recommend explaining in the docs for the getters that if you
 overwrite them, it should be to replace the setting of the fields directly
 on the ModelAdmin

--
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.b1471544b339bdb2fd9d331f7643e95d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Accepted
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--
Changes (by timo):

 * cc: timo (added)
 * stage:  Unreviewed => Accepted


Comment:

 I guess this hasn't come up before because I think most apps probably
 don't define both the attribute and the method versions, but I can see it
 could be useful.

 I am not sure if the overhead of copying the attribute all the time is
 worth it (since it probably doesn't matter for most users), but if not, we
 should at least document the caveat.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.76cf6c754196734f2a7090646db25a45%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by vzima):

 I see more than a few examples.

 Example 1: You can have admin, who can edit all of the user's data, and
 people from personal department, who can't edit username and permissions.
 {{{
 #!python
 class UserAdmin(ModelAdmin):
 readonly_fields = ['uuid'] # There can be other fields, e.g.
 identifiers to some other databases
 def get_readonly_fields(self, request, obj=None):
 readonly = super(UserAdmin, self).get_readonly_fields(request,
 obj=obj)
 if is_admin(request.user):
 return readonly
 elif is_personal_dept(request.user):
 readonly += ['permissions', 'groups'] # This will edit the
 class, so it will become readonly for everybody
 return readonly
 # Methods `has_*_permission` are modified appropriately
 }}}

 Example 2: Anything that needs somebody else's approval, e.g. vacation.
 {{{
 #!python
 class VacationAdmin(ModelAdmin):
 readonly_fields = ['uuid', 'fields_for_accounting']
 def get_readonly_fields(self, request, obj=None):
 readonly = super(VacationAdmin, self).get_readonly_fields(request,
 obj=obj)
 if is_admin(request.user) or is_boss(request.user):
 return readonly
 else:
 readonly.append('approved')
 return readonly
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.9b8dc0899081e49b2bd4b9f198f69277%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by ericpauley):

 Good point. I can't imagine any time when this would be unwanted behavior.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.46a2568893b12dcf75afc6c545178af9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--

Comment (by bmispelon):

 Hi,

 I'm ont too convinced that putting a bunch of `copy.deepcopy` is a viable
 solution to this.

 What's your use-case for manipulating `ModelAdmin` instances in such a
 fashion? It doesn't seem very typical to me.

 Thanks.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.81f084627e3ff1543d2623208ea96159%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+--
 Reporter:  vzima  |Owner:  ericpauley
 Type:  Bug|   Status:  assigned
Component:  contrib.admin  |  Version:  master
 Severity:  Normal |   Resolution:
 Keywords: | Triage Stage:  Unreviewed
Has patch:  0  |  Needs documentation:  0
  Needs tests:  0  |  Patch needs improvement:  0
Easy pickings:  0  |UI/UX:  0
---+--
Changes (by ericpauley):

 * owner:  nobody => ericpauley
 * needs_docs:   => 0
 * status:  new => assigned
 * needs_tests:   => 0
 * needs_better_patch:   => 0


Comment:

 I'm taking a look at this and trying to figure out the best way to deal
 with `get_fieldsets()`, since just copying the list wouldn't really solve
 the problem. Could `copy.deepcopy()` just be used or would that cause
 problems with copying too deep?

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/063.cfa3dcdf0d2d1e89e9be10c0d1a6fbe7%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

[Django] #22828: Model admins should return copies of its attributes

[Django]

#22828: Model admins should return copies of its attributes
---+
 Reporter:  vzima  |  Owner:  nobody
 Type:  Bug| Status:  new
Component:  contrib.admin  |Version:  master
 Severity:  Normal |   Keywords:
 Triage Stage:  Unreviewed |  Has patch:  0
Easy pickings:  0  |  UI/UX:  0
---+
 I found out there is hidden problem in `ModelAdmin`s. Getters returns
 directly attributes and not their copies. This has a hidden danger in
 cases when you override the getters. You can easily change the
 configuration of the model admin.

 Example:
 {{{
 #!python
 from django.contrib.admin.options import BaseModelAdmin
 class MyAdmin(BaseModelAdmin):
 readonly_fields = ['foo', ]

 admin = MyAdmin()
 rf = admin.get_readonly_fields(None)
 # For example, but it can very easily happen in the method override.
 rf.append('bar')
 MyAdmin.readonly_fields #>>> ['foo', 'bar']
 }}}

 === Affected attributes ===
  * fieldsets
  * fileds
  * ordering
  * readonly_fields
  * prepopulated_fields
  * list_display
  * list_display_links
  * list_filter
  * search_fields


 Django should return copies in getters of these attributes to avoid
 unwanted changes of the `ModelAdmin` at runtime.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-updates+unsubscr...@googlegroups.com.
To post to this group, send email to django-updates@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/048.43beff7a0a4ef850fc42d3ef479456f9%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.