Opened 10 years ago

Closed 10 years ago

#24502 closed Uncategorized (needsinfo)

Using non model fields in an admin ModelForm does not work if the field is listed in fieldsets (and fields I think)

Reported by: Benjamin Dauvergne Owned by: nobody
Component: Uncategorized Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have such change form:

 19 class UserAttributeFormMixin(object):
 20     def __init__(self, *args, **kwargs):
 21         super(UserAttributeFormMixin, self).__init__(*args, **kwargs)
 22         self.attributes = self.get_attributes()
 23         initial = {}
 24         if 'instance' in kwargs:
 25             content_type = ContentType.objects.get_for_model(self.instance)
 26             for av in models.AttributeValue.objects.filter(
 27                     content_type=content_type,
 28                     object_id=self.instance.pk):
 29                 initial[av.attribute.name] = av.to_python()
 30         for attribute in self.attributes:
 31             iv = initial.get(attribute.name)
 32             attribute.contribute_to_form(self, initial=iv)
 33 

class UserChangeForm(forms.UserAttributeFormMixin,
 11         AuthUserChangeForm):
 12 
 13     class Meta(AuthUserChangeForm.Meta):
 14         model = get_user_model()
 15         fields = '__all__'
 16 

Use by the following ModelAdmin:

161 class AuthenticUserAdmin(UserAdmin):
162     fieldsets = (
163         (None, {'fields': ('username', 'password')}),
164         (_('Personal info'), {'fields': ('first_name', 'last_name', 'email')}),
165         (_('Permissions'), {'fields': ('is_active', 'is_staff', 'is_superuser',
166                                        'groups')}),
167         (_('Important dates'), {'fields': ('last_login', 'date_joined')}),
168     )
169     form = admin_forms.UserChangeForm
170     add_form = admin_forms.UserCreationForm
171     add_fieldsets = (
172             (None, {
173                 'classes': ('wide',),
174                 'fields': ('username', 'first_name', 'last_name', 'email', 'password1', 'password2')}
175             ),
176         )
177     list_filter = UserAdmin.list_filter + (UserRealmListFilter,ExternalUserListFilter)
178 
179     def get_fieldsets(self, request, obj=None):
180         fieldsets = deepcopy(super(AuthenticUserAdmin, self).get_fieldsets(request, obj))
181         if obj:
182             if not request.user.is_superuser:
183                 fieldsets[2][1]['fields'] = filter(lambda x: x !=
184                         'is_superuser', fieldsets[2][1]['fields'])
185             qs = models.Attribute.objects.all()
186             insertion_idx = 2
187         else:
188             qs = models.Attribute.objects.filter(required=True)
189             insertion_idx = 1
190         if qs.exists():
191             fieldsets = list(fieldsets)
192             fieldsets.insert(insertion_idx,·
193                     (_('Attributes'), {'fields': [at.name for at in qs]}))
194         return fieldsets

I get the following traceback I did not get with Django 1.5 (I cannot say if it was already the case with Django 1.6 we skiped it):

/home/bdauvergne/.virtualenvs/authentic/local/lib/python2.7/site-packages/django/contrib/admin/options.py in changeform_view

                        'name': force_text(opts.verbose_name), 'key': escape(object_id)})

                if request.method == 'POST' and "_saveasnew" in request.POST:

                    return self.add_view(request, form_url=reverse('admin:%s_%s_add' % (

                        opts.app_label, opts.model_name),

                        current_app=self.admin_site.name))

            ModelForm = self.get_form(request, obj)

    ...

            if request.method == 'POST':

                form = ModelForm(request.POST, request.FILES, instance=obj)

                if form.is_valid():

                    form_validated = True

                    new_object = self.save_form(request, form, change=not add)

                else:

▶ Local vars
/home/bdauvergne/.virtualenvs/authentic/local/lib/python2.7/site-packages/django/contrib/auth/admin.py in get_form

            """

            Use special form during user creation

            """

            defaults = {}

            if obj is None:

                defaults['form'] = self.add_form

            defaults.update(kwargs)

            return super(UserAdmin, self).get_form(request, obj, **defaults)

    ...

        def get_urls(self):

            from django.conf.urls import patterns

            return patterns('',

                (r'^(\d+)/password/$',

                 self.admin_site.admin_view(self.user_change_password))

▶ Local vars
/home/bdauvergne/.virtualenvs/authentic/local/lib/python2.7/site-packages/django/contrib/admin/options.py in get_form

            if defaults['fields'] is None and not modelform_defines_fields(defaults['form']):

                defaults['fields'] = forms.ALL_FIELDS

            try:

                return modelform_factory(self.model, **defaults)

            except FieldError as e:

                raise FieldError('%s. Check fields/fieldsets/exclude attributes of class %s.'

                                 % (e, self.__class__.__name__))

    ...

        def get_changelist(self, request, **kwargs):

            """

            Returns the ChangeList class for use on the changelist page.

            """

            from django.contrib.admin.views.main import ChangeList

▶ Local vars 

What happens is that ModelAdmin is passing all fields defined to modelform_factory which use them to set the Meta.fields property of a new modelform class and it breaks with a new validation of corrects Meta.fields in django/forms/models.py l.294:

 280 
 281             fields = fields_for_model(opts.model, opts.fields, opts.exclude,
 282                                       opts.widgets, formfield_callback,
 283                                       opts.localized_fields, opts.labels,
 284                                       opts.help_texts, opts.error_messages)
 285 
 286             # make sure opts.fields doesn't specify an invalid field
 287             none_model_fields = [k for k, v in six.iteritems(fields) if not v]
 288             missing_fields = (set(none_model_fields) -
 289                               set(new_class.declared_fields.keys()))
 290             if missing_fields:
 291                 message = 'Unknown field(s) (%s) specified for %s'
 292                 message = message % (', '.join(missing_fields),
 293                                      opts.model.__name__)
 294                 raise FieldError(message)
 295             # Override default model fields with any custom declared ones
 296             # (plus, include all the other declared fields).
 297             fields.update(new_class.declared_fields)
 298         else:

Don't know it it's better to allow non-model fields in Meta.fields or to fix the admin.

Change History (2)

comment:1 by Benjamin Dauvergne, 10 years ago

I worked around the problem with this hack in my code:

 
@@ -188,16 +189,30 @@ class AuthenticUserAdmin(UserAdmin):
             qs = models.Attribute.objects.filter(required=True)
             insertion_idx = 1
         if qs.exists():
             fieldsets = list(fieldsets)
             fieldsets.insert(insertion_idx, 
                     (_('Attributes'), {'fields': [at.name for at in qs]}))
         return fieldsets
 
+    def get_form(self, request, obj=None, **kwargs):
+        if 'fields' in kwargs:
+            fields = kwargs.pop('fields')
+        else:
+            fields = flatten_fieldsets(self.get_fieldsets(request, obj))
+        if obj:
+            qs = models.Attribute.objects.all()
+        else:
+            qs = models.Attribute.objects.filter(required=True)
+        non_model_fields = [a.name for a in qs]
+        fields = list(set(fields) - set(non_model_fields))
+        kwargs['fields'] = fields
+        return super(AuthenticUserAdmin, self).get_form(request, obj=obj, **kwargs)
+

comment:2 by Tim Graham, 10 years ago

Resolution: needsinfo
Status: newclosed

Your example is too complicated for me to debug and reproduce (for example attribute.contribute_to_form isn't part of Django), and there's evidence that referring to non-model fields does work, see https://github.com/django/django/commit/dc3d2ac98c1bcfad74d3e9523caf07e7e9fb15aa for tests. Please reopen if you can simplify your example so it can be reproduced (ideally, as a test for Django's test suite that passes on 1.5 and fails in 1.7). If you can bisect the regression all the better.

Note: See TracTickets for help on using tickets.
Back to Top