Opened 11 years ago

Closed 18 months ago

#20942 closed Bug (duplicate)

modelformset wrongly assumes foreign keys can't point to proxy models.

Reported by: Craig de Stigter Owned by: nobody
Component: Forms Version: 1.5
Severity: Normal Keywords:
Cc: charlesroelli Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Background: My reusable app django-typed-models extends proxy models with a type field, so the proxy model for each instance gets stored in the database.

I wanted to have a ForeignKey pointing to one of those proxy models. The database level behaviour is the same (it points to the concrete model that the proxy model proxies), but this makes Django do better type checking for me:

    class Animal(TypedModel):
      name = models.CharField()

    class Canine(Animal):
      pass

    class Feline(Animal):
      pass

    class BigCat(Feline):
      pass

    class OtherModel(models.Model):
        # at DB level this points to Animal
        cat = models.ForeignKey(Feline)

The idea being that this will work fine:

    othermodel.cat = BigCat(name="simba")

But this will throw an error:

    othermodel.cat = Canine(name="fido")
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/cdestigter/checkout/django/django/db/models/fields/related.py", line 405, in __set__
        self.field.name, self.field.rel.to._meta.object_name))
    ValueError: Cannot assign "<Canine: fido>": "OtherModel.cat" must be a "Feline" instance.

This *almost* works ;)

In fact, it works everywhere except in the admin. I get these errors:

    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/usr/lib/python2.6/dist-packages/dandelion/geostore/admin.py", line 7, in <module>
        admin.autodiscover()
      File "/home/cdestigter/checkout/django/django/contrib/admin/__init__.py", line 29, in autodiscover
        import_module('%s.admin' % app)
      File "/home/cdestigter/checkout/django/django/utils/importlib.py", line 35, in import_module
        __import__(name)
      File "/usr/lib/python2.6/dist-packages/dandelion/datasources/admin.py", line 183, in <module>
        import_module(admin_module)
      File "/home/cdestigter/checkout/django/django/utils/importlib.py", line 35, in import_module
        __import__(name)
      File "/usr/lib/python2.6/dist-packages/myproject/myapp/admin.py", line 18, in <module>
        admin.site.register(BigCat, BigCatOptions)
      File "/home/cdestigter/checkout/django/django/contrib/admin/sites.py", line 98, in register
        validate(admin_class, model)
      File "/home/cdestigter/checkout/django/django/contrib/admin/validation.py", line 189, in validate
        validate_inline(inline, cls, model)
      File "/home/cdestigter/checkout/django/django/contrib/admin/validation.py", line 200, in validate_inline
        fk = _get_foreign_key(parent_model, cls.model, fk_name=cls.fk_name, can_fail=True)
      File "/home/cdestigter/checkout/django/django/forms/models.py", line 807, in _get_foreign_key
        raise Exception("fk_name '%s' is not a ForeignKey to %s" % (fk_name, parent_model))
    Exception: fk_name 'cat' is not a ForeignKey to <class 'myproject.myapp.models.BigCat'>

I have a small patch which fixes the issue. I can add tests to it if someone can accept the concept.

Attachments (1)

20942-no-tests.diff (1.2 KB ) - added by Craig de Stigter 11 years ago.

Download all attachments as: .zip

Change History (6)

by Craig de Stigter, 11 years ago

Attachment: 20942-no-tests.diff added

comment:1 by Craig de Stigter, 11 years ago

Has patch: set
Needs tests: set
Patch needs improvement: set
Type: UncategorizedBug

comment:2 by Kevin Christopher Henry, 11 years ago

Triage Stage: UnreviewedAccepted

comment:3 by charlesroelli, 18 months ago

Cc: charlesroelli added

comment:4 by OnurBilgic, 18 months ago

With @charlesroelli we found that the Django admin works with foreign keys pointing to proxy models. The problem was fixed in #32975. The admin error has not existed anymore. The example code above is manually tested with TypedModel https://github.com/craigds/django-typed-models library. It works well.

comment:5 by Claude Paroz, 18 months ago

Resolution: duplicate
Status: newclosed

Thanks a lot for checking this!

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