Code

Opened 6 years ago

Closed 2 years ago

#8394 closed Bug (needsinfo)

ModelForm subclasses act differently to ModelForms even if no changes are made

Reported by: devinj Owned by: nobody
Component: Forms Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I think this is a consequence of the metaclass it has, and it may not even be something that should be fixed-- maybe this is intended, and I'm doing something stupid. Anyway, I was messing around with ModelAdmin, and wanted to customize the forms for a large group of them, in a common way-- I figured, alright, I can set the form in a superclass. It's normally ModelAdmin, with no Meta inner class, and no model defined, so it should be easy to define on my own without having to redo it with a different Meta inner class for each of my models, right? The problem is that this results in errors (fields that exist supposedly not existing). If I define a

class MyModelAdmin(ModelAdmin): form = ModelForm

, everything works fine. If I define a

class MyModelForm(ModelForm): pass
class MyModelAdmin(ModelAdmin): form = MyModelForm

it breaks. The workaround I tried to use was to, instead of inheriting from ModelForm, create a new ModelForm in parallel (inherit from BaseModelForm and set the metaclass). The only problem is that this means ModelForm is not in the inheritance tree, which means that the metaclass doesn't do what it should (leading to errors about _meta), which means that the only solution I can do is to rewrite (that is, copy-paste and change a single line) the metaclass to instead refer to my form. I believe, in fact, that this metaclass is the whole reason for the problem-- it does something special for ModelForm, but not so for ModelForm subclasses (or perhaps it's the reverse). Attached is an example of this ugly and very flawed (it sort of works, but something's wrong with the media and so on, making the date and time field worthless) workaround, using step two of the tutorial. I really don't know much about Django, I only started a few weeks ago, so this is diving pretty deep into the innards for me. I'd guess maybe some sort of marker saying "hey, I'm just like ModelForm" (abstract = True?) would be great. All I really know is that I don't like the way it is now, since the only sensible way to get around this is to individually define my ModelForms with the Meta class included-- so far as I can tell.

Attachments (1)

models.py (2.6 KB) - added by devinj 6 years ago.

Download all attachments as: .zip

Change History (6)

Changed 6 years ago by devinj

comment:1 Changed 6 years ago by jarrow

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Up front: My comment does only speak about the admin. I haven't tested the issue outside of it ...

This looks like the issue described here #8299. Try adding the Meta: model = MyModel part to you ModelForm subclass. If it still breaks a minimal example would be really helpful.

I actually looked at this some more after filing #8299. I think the only thing that breaks if you don't add a Meta class to your ModelForm subclass is the validation of the fieldsets attribute you provide (and maybe for other attributes). The actual admin works! When DEBUG is True it is checked that the fields you provide in the fieldsets do exist in the form. But this is not the actual form that is used in the admin. As far as I know the admin always uses get_form() which adds all necessary information to the subclass.

So I tried fixing the validation but that would pretty much require to move the stuff that's hapening in the implementation of get_form() to some static function and calling that in the validation code. I though this might be to great a change but still it would be possible to fix this (in the admin at least). See validation code at: http://code.djangoproject.com/browser/django/trunk/django/contrib/admin/validation.py#L275

comment:2 Changed 5 years ago by jacob

  • Triage Stage changed from Unreviewed to Design decision needed

comment:3 Changed 5 years ago by jacob

I've certainly written code that works with form subclasses -- in fact, that's the whole point of the form bit. Can you please clarify what "breaks" means? Post the traceback? I suspect this is something specific to your use case, and perhaps a bug in your code, not in Django.

comment:4 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:5 Changed 2 years ago by aaugustin

  • Easy pickings unset
  • Resolution set to needsinfo
  • Status changed from new to closed
  • UI/UX unset

The report is quite confusing :|

Closing since there wasn't an answer to Jacob's comment.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.