Opened 4 years ago

Closed 3 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)

Change History (13)

comment:1 Changed 4 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to needsinfo
  • Status changed from new to closed

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 Changed 4 years ago by anonymous

  • Resolution needsinfo deleted
  • Status changed from closed to reopened

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 Changed 4 years ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

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

comment:4 Changed 4 years ago by volrath

  • Has patch set
  • UI/UX unset

comment:5 Changed 4 years ago by julien

  • Needs tests set

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

comment:6 Changed 4 years ago by volrath

  • 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 Changed 4 years ago by julien

  • 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 Changed 4 years ago by volrath

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 Changed 3 years ago by vanschelven

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 Changed 3 years ago by claudep

  • Status changed from reopened to new
  • Triage Stage changed from Accepted to Ready for checkin

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

  • Resolution set to fixed
  • Status changed from new to closed

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