Opened 13 years ago

Closed 11 years ago

#15877 closed Bug (fixed)

Exception:ModelForm has no model class specified

Reported by: theaspect@… Owned by: nobody
Component: Forms Version: 1.3
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

case:

subj exception does not throws when

in user class model is not specified
in constructor instance is specified

result:

silently falls in validation returns > empty cleaned dict > nothing rendered

expected:

exception throws

here: http://code.djangoproject.com/browser/django/trunk/django/forms/models.py#L229

Attachments (2)

0001-Fixes-15877-ModelForm-will-raise-ValueError-if-not-M.patch (3.5 KB ) - added by Daniel Barreto 12 years ago.
0001-Fixes-15877-ModelForm-will-raise-ValueError-if-not-M.2.patch (2.3 KB ) - added by Daniel Barreto 12 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by Anssi Kääriäinen, 13 years ago

Resolution: needsinfo
Status: newclosed

I am afraid that there is not enough information about the problem to verify this as a bug. Giving concrete sample of the problem would help. That is, could you write Python code which would show the problem?

comment:2 by anonymous, 13 years ago

Resolution: needsinfo
Status: closedreopened

it's pretty clear

class SomeModel(models.Model):
    ...

class MyForm(forms.ModelForm):
    class Meta:
        pass

f = MyForm() #raises exception (ModelForm has no model class specified.)
f = MyForm(instance=SomeModel) #don't but expected too

comment:3 by Jacob, 13 years ago

Triage Stage: UnreviewedAccepted

Verified. ModelForm.__init__ should raise the ValueError in all cases.

comment:4 by Daniel Barreto, 12 years ago

Has patch: set
UI/UX: unset

comment:5 by Julien Phalip, 12 years ago

Needs tests: set

Thanks for the patch. Could you also provide some tests?

comment:6 by Daniel Barreto, 12 years ago

Needs tests: unset

I replaced previous patch with a new one that includes tests.

There's one thing I'm not sure it's right: if you look the patch you'll see I deleted part of a test (OldFormForXTests.test_with_data) that's against the "ModelForm.__init__ should raise the ValueError in all cases" restriction, and of course it was making that test fail. Please tell me if that should not be deleted so I can fix it.

comment:7 by Julien Phalip, 12 years ago

Patch needs improvement: set

The new test looks good. However, there's no need to remove the failing existing test. That could be fixed by adding 'class Meta: model = Category' to ShortCategory to make it valid.

comment:8 by Daniel Barreto, 12 years ago

I added a new version of the patch.

I thought of that, but it seemed to me (from the comments on that part of the test) they were trying to test that even though ShortCategory didn't have a model class, having the same fields as the Category model was good enough to save the form using a base category instance.

Anyway, There's the new patch with your correction.

comment:9 by Klaas van Schelven, 11 years ago

The above story from 16 months ago checks out, I have no idea why it was never committed. I've reimplemented it as a pull request.

https://github.com/django/django/pull/783

comment:10 by Claude Paroz, 11 years ago

Status: reopenednew
Triage Stage: AcceptedReady for checkin

comment:11 by Claude Paroz <claude@…>, 11 years ago

Resolution: fixed
Status: newclosed

In cc53d9b30bd73c12413c28101d5db9f9f4df517c:

Fixed #15877 -- Improved exception when ModelForm has no model class

Thanks theaspect at gmail.com for the report and volrath for the
patch.

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