-
Notifications
You must be signed in to change notification settings - Fork 60
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
Error when loading in wagtailtrans own hooks #153
base: master
Are you sure you want to change the base?
Conversation
In what case would you want a page to have a language object in your translated tree and not inherit from translatable page.. seems a bit weird too me when having a dutch tree to create an article in french right? Can you elaborate a bit more on this issue? |
Hi @OktayAltay , In PR #155 there is some work done to have a TranslatableMixin in the future, this would also make it possible to have other content translated via wagtailtrans, which would be really nice to have. However this change also allows you to not have the |
We could change those lines to reference Personally, I prefer the |
I have swapped the |
self.article = ArticleFactory() | ||
self.transpage = TranslatablePage() | ||
|
||
@hooks.register('register_page_listing_buttons') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you defining this here? I think we can just import it right, just like the edit_in_language_button
, now you're testing code which is defined in the test, seems a bit weird..
|
||
@pytest.mark.django_db | ||
class TestWagtailHooks: | ||
def setup(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the 2 pages created in the setup for each test, so lets just create the page we need in the test itself
@pytest.mark.django_db | ||
class TestWagtailHooks: | ||
def setup(self): | ||
self.article = ArticleFactory() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose of this ArticlePage
is to have a page with a relation to language which doesn't extend from TranslatablePage
right? so lets call it: NormalPageWithLanguage
or something like that so we have a more clear view for what purpose this page is created
@@ -74,3 +77,35 @@ class WagtailPageFactory(factory.DjangoModelFactory): | |||
|
|||
class Meta: | |||
model = Page | |||
|
|||
@classmethod | |||
def _create(cls, *args, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think we need this since we're mostly using a pytest fixture when creating a site tree. Or is there any particular reason why you implemented this?
import factory | ||
from factory.fuzzy import FuzzyDate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
factory.fuzzy is deprecated according to the docs: https://factoryboy.readthedocs.io/en/latest/fuzzy.html
@@ -116,7 +115,7 @@ def edit_in_language_button(page, page_perms, is_parent=False): | |||
clear interface to work in. | |||
|
|||
""" | |||
if not hasattr(page, 'language'): | |||
if not issubclass(page.__class__, TranslatablePage): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry, my last comment was misleading. I meant isinstance
, as you had changed it just before, not sure why I said issubclass
!
Thanks for contributing!
Hi, when using wagtailtrans in combination with a Wagtail
Page
model in a specific way will generatethe following error 'model object has no attribute 'get_translations'.
To reproduce this you only need to use wagtail trans and also having a page model for example called
Article
that uses Wagtails own Page model and not theTranslatablePage
. ThatArticle
model must have a foreignkey field named language that is linked to wagtailtransLanguage
model.To trigger the error you just have to go to the admin page and visit the
Article
page. Wagtail will then try loading wagtailtrans own hooksedit_in_language_button
orpage_translations_menu
this is based on your settings. this will than generate that error.