Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
timheap opened this issue Jul 25, 2014 · 6 comments

Comments

Projects
None yet
4 participants
@timheap
Copy link
Contributor

commented Jul 25, 2014

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

This comment has been minimized.

Copy link
Member

commented Jul 25, 2014

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()<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
#503.

@davecranwell

This comment has been minimized.

Copy link
Collaborator

commented Jul 25, 2014

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.

@timheap

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2014

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()<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
#503.


Reply to this email directly or view it on GitHub:
#503 (comment)

@timheap

This comment has been minimized.

Copy link
Contributor Author

commented Jul 29, 2014

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 kaedroho added the wontfix label Aug 15, 2014

@kaedroho

This comment has been minimized.

Copy link
Member

commented Aug 15, 2014

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).

@timheap timheap closed this Aug 17, 2014

@kaedroho kaedroho removed the wontfix label Feb 17, 2015

@kaedroho kaedroho reopened this Feb 18, 2015

@kaedroho

This comment has been minimized.

Copy link
Member

commented Feb 18, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.