wagtail / wagtail

A Django content management system focused on flexibility and user experience
https://wagtail.org
BSD 3-Clause "New" or "Revised" License
17.98k stars 3.79k forks source link

Can not have a field named the same as a Page model #503

Open mx-moth opened 10 years ago

mx-moth commented 10 years ago

The following model definition will cause an error when instansiating a Foo:

from django.db import models
from wagtail.wagtailcore.models import Page

class Foo(Page):
    bar = models.CharField(max_length=255)

class Bar(Page):
    pass
$ ./manage.py shell
Python 2.7.3 (default, Mar 13 2014, 11:03:55) 
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from wagtailtests.website.models import Foo, Bar
>>> Bar()
<Bar: >
>>> Foo()
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/vagrant/wagtail/wagtailcore/models.py", line 305, in __init__
    super(Page, self).__init__(*args, **kwargs)
  File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/modelcluster/models.py", line 114, in __init__
    super(ClusterableModel, self).__init__(*args, **kwargs)
  File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 407, in __init__
    setattr(self, field.attname, val)
  File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 223, in __set__
    self.related.get_accessor_name(), self.related.opts.object_name))
ValueError: Cannot assign "u''": "Foo.bar" must be a "Bar" instance.

This does not happen when using django.db.models.Model as the base class of Foo and Bar, so this is a Wagtail issue. I encountered this issue when attempting to create a Page model named Website, and had another Page model with a field website.

kaedroho commented 10 years ago

Thanks for spotting!

This sounds like a clash with the virtual field Django provides for finding the "Bar" instance for a page (if there is one).

I'm not sure how this could be solved at the moment. But I would recommend suffixing your page types with "Page" to prevent clashes like this happening.

On 25 July 2014 02:44, Tim Heap notifications@github.com wrote:

The following model definition will cause an error when instansiating a Foo:

from django.db import modelsfrom wagtail.wagtailcore.models import Page

class Foo(Page): bar = models.CharField(max_length=255)

class Bar(Page): pass

$ ./manage.py shellPython 2.7.3 (default, Mar 13 2014, 11:03:55) [GCC 4.7.2] on linux2Type "help", "copyright", "credits" or "license" for more information.(InteractiveConsole)>>> from wagtailtests.website.models import Foo, Bar>>> Bar()>>> Foo()Traceback (most recent call last): File "", line 1, in File "/vagrant/wagtail/wagtailcore/models.py", line 305, in init super(Page, self).init(_args, _kwargs) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/modelcluster/models.py", line 114, in init super(ClusterableModel, self).init(_args, _kwargs) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 407, in init setattr(self, field.attname, val) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 223, in set self.related.get_accessor_name(), self.related.opts.object_name))ValueError: Cannot assign "u''": "Foo.bar" must be a "Bar" instance.

This does not happen when using django.db.models.Model as the base class of Foo and Bar, so this is a Wagtail issue. I encountered this issue when attempting to create a Page model named Website, and had another Page model with a field Website.

— Reply to this email directly or view it on GitHub https://github.com/torchbox/wagtail/issues/503.

davecranwell commented 10 years ago

What @kaedroho said. Almost every field name we use in our clients' sites could just as easily describe a whole model required by another part of the project, so we've taken to prefixing all of our page models, whether we foresee a clash or not.

mx-moth commented 10 years ago

Ah yes, that would explain it. You're suggested work around would work. This probably needs documenting somewhere though, because it confused the hell out of me! Would it be possible/sensible to disable this relation? I can't see it being used often as Page is a generic base class, so you would not often go from that to a specific page type.

On 25 July 2014 6:43:35 PM AEST, Karl Hobley notifications@github.com wrote:

Thanks for spotting!

This sounds like a clash with the virtual field Django provides for finding the "Bar" instance for a page (if there is one).

I'm not sure how this could be solved at the moment. But I would recommend suffixing your page types with "Page" to prevent clashes like this happening.

On 25 July 2014 02:44, Tim Heap notifications@github.com wrote:

The following model definition will cause an error when instansiating a Foo:

from django.db import modelsfrom wagtail.wagtailcore.models import Page

class Foo(Page): bar = models.CharField(max_length=255)

class Bar(Page): pass

$ ./manage.py shellPython 2.7.3 (default, Mar 13 2014, 11:03:55) [GCC 4.7.2] on linux2Type "help", "copyright", "credits" or "license" for more information.(InteractiveConsole)>>> from wagtailtests.website.models import Foo, Bar>>> Bar()>>> Foo()Traceback (most recent call last): File "", line 1, in File "/vagrant/wagtail/wagtailcore/models.py", line 305, in init super(Page, self).init(_args, _kwargs) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/modelcluster/models.py", line 114, in init super(ClusterableModel, self).init(_args, _kwargs) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/base.py", line 407, in init setattr(self, field.attname, val) File "/home/vagrant/wagtailtests_venv/local/lib/python2.7/site-packages/django/db/models/fields/related.py", line 223, in set self.related.get_accessor_name(), self.related.opts.object_name))ValueError: Cannot assign "u''": "Foo.bar" must be a "Bar" instance.

This does not happen when using django.db.models.Model as the base class of Foo and Bar, so this is a Wagtail issue. I encountered this issue when attempting to create a Page model named Website, and had another Page model with a field Website.

— Reply to this email directly or view it on GitHub https://github.com/torchbox/wagtail/issues/503.


Reply to this email directly or view it on GitHub: https://github.com/torchbox/wagtail/issues/503#issuecomment-50123069

mx-moth commented 10 years ago

I managed to work around this issue by overriding the automatically generated OneToOneField for Multi-table inheritance, and disabling the reverse relation:

from django.db import models
from wagtail.wagtailcore.models import Page

class Foo(Page):
    page_ptr = models.OneToOneField(Page, parent_link=True, related_name='+')
    bar = models.CharField(max_length=255)

class Bar(Page):
    page_ptr = models.OneToOneField(Page, parent_link=True, related_name='+')

page_ptr is the name Django automatically generates for this field. Keeping it the same in the overridden field should stop anything strange from happening. I am trying to see if there is any simple way of placing this on all models automatically through subclassing or some such to disable this application wide.

kaedroho commented 10 years ago

Thanks @timheap

This wouldn't be easy to fix in Wagtail (it's more of a Django issue anyway) and can be easily worked around by suffixing your page models with -Page (and also using the method you suggested).

kaedroho commented 9 years ago

I've found another issue recently that is related to this.

It is not possible to have two page classes with the same name, even if they're in different apps. This is because the reverse accessor from the Page class would be given the same name causing a clash.

This is possible to work around using the code above to override page_ptr. It is not possible to work around it if your page type decends from an abstract class (form pages for example). The only thing you can do is rename your models.