Opened 2 years ago

Last modified 2 years ago

#33703 closed Uncategorized

ModelAdmin.get_fields wrong work with ModelAdmin.fieldsets statement — at Version 1

Reported by: Maxim Danilov Owned by: nobody
Component: contrib.admin Version: 4.0
Severity: Normal Keywords: admin, modeladmin
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Maxim Danilov)

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.

Change History (2)

by Maxim Danilov, 2 years ago

Attachment: test.py added

example which works for me on django 4.01

comment:1 by Maxim Danilov, 2 years ago

Description: modified (diff)
Easy pickings: unset
Has patch: unset
Note: See TracTickets for help on using tickets.
Back to Top