#33703: ModelAdmin.get_fields wrong work with ModelAdmin.fieldsets statement
-----------------------------------+--------------------------------------
     Reporter:  Maxim Danilov      |                    Owner:  nobody
         Type:  Uncategorized      |                   Status:  new
    Component:  contrib.admin      |                  Version:  4.0
     Severity:  Normal             |               Resolution:
     Keywords:  admin, modeladmin  |             Triage Stage:  Unreviewed
    Has patch:  0                  |      Needs documentation:  0
  Needs tests:  0                  |  Patch needs improvement:  0
Easy pickings:  0                  |                    UI/UX:  0
-----------------------------------+--------------------------------------
Changes (by Maxim Danilov):

 * has_patch:  1 => 0
 * easy:  1 => 0


Old description:

> almost all my issue is "not fixed", but i try
>
> I found a bug: ModelAdmin.get_fields works wrong with fieldset statement.
>
> How to reproduce it:
>
> {{{
> from django.db import models
> from django.contrib.admin import ModelAdmin
>
> class MyModel(models.Model):
>     title = models.CharField('Meta Title of object', max_length=80,
> blank=True)
>     slug = models.CharField('Meta Slug of object', max_length=80,
> blank=True)
>
> class MyModelAdmin1(ModelAdmin):
>
>     fields = ('title', )
>

> class MyModelAdmin2(ModelAdmin):
>     fieldset = ('title', )
>

> print(MyModelAdmin1(MyModel, None).get_fields(None))
> print(MyModelAdmin2(MyModel, None).get_fields(None))
> }}}
> save as test.py, in new or old project.
> python manage.py test output:
>
> {{{
> ('title',)
> ['title', 'slug']
> Found xxx test(s).
> ...
> }}}
>
> Problem is nor directly in def get_fields.
> {{{
>     def get_fields(self, request, obj=None):
>         """
>         Hook for specifying fields.
>         """
>         if self.fields:
>             return self.fields
>         # _get_form_for_get_fields() is implemented in subclasses.
>         form = self._get_form_for_get_fields(request, obj)
>         return [*form.base_fields, *self.get_readonly_fields(request,
> obj)]
> }}}
>
> But in call of self._get_form_for_get_fields
>
> {{{
>     def _get_form_for_get_fields(self, request, obj):
>         return self.get_form(request, obj, fields=None)
> }}}
>
> The author of _get_form_for_get_fields has forgotten how self.get_form
> handles "fields":
>
> {{{
>     def get_form(self, request, obj=None, change=False, **kwargs):
>         """
>         Return a Form class for use in the admin add view. This is used
> by
>         add_view and change_view.
>         """
>         if 'fields' in kwargs:
>             fields = kwargs.pop('fields')
>         else:
>             fields = flatten_fieldsets(self.get_fieldsets(request, obj))
>         ...
> }}}
>
> I see two possibility to fix it:
>
> or
> {{{
>     def _get_form_for_get_fields(self, request, obj):
>         return self.get_form(request, obj)
> }}}
>
> or
>
> {{{
>     def get_form(self, request, obj=None, change=False, **kwargs):
>         """
>         Return a Form class for use in the admin add view. This is used
> by
>         add_view and change_view.
>         """
>         fields = kwargs.pop('fields', None) or []
>         if not fields:
>             fields = flatten_fieldsets(self.get_fieldsets(request, obj))
>         ...
> }}}
> p.s.
> this issue is easy to create, we already have 'contrib.admin' in
> "component select" in Issue-Creation-Form, unlike, for example,
> 'contrib.gis'.
> For 'contrib.gis' we suddenly use GIS in uppercase.

New description:

 almost all my issue is "not fixed", but i try

 I found a bug: ModelAdmin.get_fields works wrong with fieldset statement.

 How to reproduce it:

 {{{
 from django.db import models
 from django.contrib.admin import ModelAdmin

 class MyModel(models.Model):
     title = models.CharField('Meta Title of object', max_length=80,
 blank=True)
     slug = models.CharField('Meta Slug of object', max_length=80,
 blank=True)

 class MyModelAdmin1(ModelAdmin):

     fields = ('title', )


 class MyModelAdmin2(ModelAdmin):
     fieldset = ('title', )


 print(MyModelAdmin1(MyModel, None).get_fields(None))
 print(MyModelAdmin2(MyModel, None).get_fields(None))
 }}}
 save as test.py, in new or old project.
 python manage.py test output:

 {{{
 ('title',)
 ['title', 'slug']
 Found xxx test(s).
 ...
 }}}

 Problem is nor directly in def get_fields.
 {{{
     def get_fields(self, request, obj=None):
         """
         Hook for specifying fields.
         """
         if self.fields:
             return self.fields
         # _get_form_for_get_fields() is implemented in subclasses.
         form = self._get_form_for_get_fields(request, obj)
         return [*form.base_fields, *self.get_readonly_fields(request,
 obj)]
 }}}

 But in call of self._get_form_for_get_fields

 {{{
     def _get_form_for_get_fields(self, request, obj):
         return self.get_form(request, obj, fields=None)
 }}}

 The author of _get_form_for_get_fields has forgotten how self.get_form
 handles "fields":

 {{{
     def get_form(self, request, obj=None, change=False, **kwargs):
         """
         Return a Form class for use in the admin add view. This is used by
         add_view and change_view.
         """
         if 'fields' in kwargs:
             fields = kwargs.pop('fields')
         else:
             fields = flatten_fieldsets(self.get_fieldsets(request, obj))
         ...
 }}}

 I thought we had two options to fix this:


 {{{
     def _get_form_for_get_fields(self, request, obj):
         return self.get_form(request, obj)
 }}}

 or

 {{{
     def get_form(self, request, obj=None, change=False, **kwargs):
         """
         Return a Form class for use in the admin add view. This is used by
         add_view and change_view.
         """
         fields = kwargs.pop('fields', None) or []
         if not fields:
             fields = flatten_fieldsets(self.get_fieldsets(request, obj))
         ...
 }}}

 BUT! My solution not works, we receive a recursion:

 {{{
 in get_fields
   call self._get_form_for_get_fields

 in _get_form_for_get_fields
   call self.get_form

 in get_form
   call self.get_fieldsets

 in get_fieldsets
   call self.get_fields (go to 1. step)
 }}}

 p.s.
 this issue is easy to create, we already have 'contrib.admin' in
 "component select" in Issue-Creation-Form, unlike, for example,
 'contrib.gis'.
 For 'contrib.gis' we suddenly use GIS in uppercase.

--

-- 
Ticket URL: <https://code.djangoproject.com/ticket/33703#comment:1>
Django <https://code.djangoproject.com/>
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 view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/01070180b99523cf-6e7bd5fd-6574-43b1-8d2d-4feb7878161d-000000%40eu-central-1.amazonses.com.

Reply via email to