Inheritance in admin classes

142 views
Skip to first unread message

kny...@knyg.ht

unread,
Jul 1, 2015, 5:32:48 PM7/1/15
to django-d...@googlegroups.com
Hello all,

So, I was talking to kezabelle on IRC some months back about a problem I was having, and finally remembered to post something.

If I inherit from an admin class, and override something like get_fields(), I was getting back duplicates of fields if I used super() to get the existing fields and add to them. And if I reload the page, I would get more and more fields until I restarted the dev server. Something about ModelAdmin being one instance per process. It seems very counter-intuitive to me, and I was wondering if anyone was considering fixing this? I'm under the impression it might be some work to do.

Cheers,
Tom

Tim Graham

unread,
Jul 1, 2015, 9:33:24 PM7/1/15
to django-d...@googlegroups.com

kny...@knyg.ht

unread,
Jul 2, 2015, 6:30:59 AM7/2/15
to django-d...@googlegroups.com
I think it could be - if not the exact same issue - one that would be fixed by the same patch. My specific use case is something like this:

class MyUserAdmin(UserAdmin):
    def get_fieldsets(self, request, obj=None):
        fieldsets = super().get_fieldsets(request, obj)
        fieldsets[0][1]['fields'] = list(fieldsets[0][1]['fields'])
        fieldsets[0][1]['fields'].append('category')

which is already pretty ugly, but needs uglifying further:

class MyUserAdmin(UserAdmin):
    def get_fieldsets(self, request, obj=None):
        fieldsets = super().get_fieldsets(request, obj)
        fieldsets[0][1]['fields'] = list(fieldsets[0][1]['fields'])

        if 'category' not in fieldsets[0][1]['fields'][-1]:
            fieldsets[0][1]['fields'].append('category')

Logically, they could be the same issue, but I haven't dug in to see if this is the case. Is it really the case that the overhead of deepcopy() is too much here?

Jani Tiainen

unread,
Jul 2, 2015, 6:46:37 AM7/2/15
to django-d...@googlegroups.com
Hi,

I had my fun when I tried to add more inline stuff to a Admin form, and I ended up doing thiskind of stuff:

https://janimagik.wordpress.com/2015/05/05/django-admin-and-inline-chaining/

I guess problem is how Django admin uses metaclass to do some magic behind the scenes.
> --
> 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-develop...@googlegroups.com.
> To post to this group, send email to django-d...@googlegroups.com.
> Visit this group at http://groups.google.com/group/django-developers.
> To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/bd375dce-428f-4ca3-a2bc-2533f47134db%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.


--
Jani Tiainen

Collin Anderson

unread,
Jul 2, 2015, 8:56:10 AM7/2/15
to django-d...@googlegroups.com
Hi All,

It seems to me this is just how class inheritance works in Python. Nothing special in Django here, it just might not be intuitive.

I do think deepcopy has too much overhead to warrant a deepcopy by default in Django for everyone, but it may help you to use it in your code.

Collin

Marc Tamlyn

unread,
Jul 2, 2015, 9:08:00 AM7/2/15
to django-d...@googlegroups.com
It's a symptom of the poor design of ModelAdmin (especially compared to View). It is just "normal" python classes, but it has methods on it which feel request specific so it's very easy to fall into the trap of thinking of it as a request level object not a process level object. Model instances, form instances, view instances etc are all request level, so it's natural to assume that modeladmin would match that. In fact it doesn't, and there's an easy class of user bugs you can create by setting things onto `self` in modeladmin methods (or in this case modifying something on self) and it leaks to following requests.

I don't think is a trivial issue to fix, the correct approach would be that we have a request level object with a slightly different level of abstraction I think - making the entire ModelAdmin request level would be problematic as it is instantiated to (for example) work out which models are available in the admin and which URLs exist - data which should be stored at process level.

Fixing this specific issue may be possible, but it's a band aid for a broken leg...

Marc

Reply all
Reply to author
Forward
0 new messages