From 68b02aecc1410b987518ac9794a3e9fc49c77641 Mon Sep 17 00:00:00 2001 From: Oktay Altay Date: Fri, 8 Feb 2019 17:43:57 +0100 Subject: [PATCH 1/5] expand the if statement and add tests --- .coveragerc | 1 - src/wagtailtrans/wagtail_hooks.py | 41 ++++++++++--------- .../_sandbox/pages/migrations/0003_article.py | 31 ++++++++++++++ tests/_sandbox/pages/models.py | 17 ++++++++ tests/factories/pages.py | 37 ++++++++++++++++- tests/unit/test_wagtail_hooks.py | 32 +++++++++++++++ 6 files changed, 137 insertions(+), 22 deletions(-) create mode 100644 tests/_sandbox/pages/migrations/0003_article.py create mode 100644 tests/unit/test_wagtail_hooks.py diff --git a/.coveragerc b/.coveragerc index 447ec319..f21fbacd 100644 --- a/.coveragerc +++ b/.coveragerc @@ -4,4 +4,3 @@ omit = src/wagtailtrans/__init__.py src/wagtailtrans/admin.py src/wagtailtrans/config.py - src/wagtailtrans/wagtail_hooks.py diff --git a/src/wagtailtrans/wagtail_hooks.py b/src/wagtailtrans/wagtail_hooks.py index e29fab73..14b7820b 100644 --- a/src/wagtailtrans/wagtail_hooks.py +++ b/src/wagtailtrans/wagtail_hooks.py @@ -42,25 +42,26 @@ def global_admin_js(): ) -if not get_wagtailtrans_setting('SYNC_TREE'): - """Only load hooks when WAGTAILTRANS_SYNC_TREE is disabled""" - - @hooks.register('register_page_listing_buttons') - def page_translations_menu(page, page_perms, is_parent=False): - if not hasattr(page, 'language'): - return - - if hasattr(page, 'canonical_page') and page.canonical_page: - return - - yield widgets.ButtonWithDropdownFromHook( - 'Translate into', - hook_name='wagtailtrans_dropdown_hook', - page=page, - page_perms=page_perms, - is_parent=is_parent, - priority=10 - ) +@hooks.register('register_page_listing_buttons') +def page_translations_menu(page, page_perms, is_parent=False): + if get_wagtailtrans_setting('SYNC_TREE'): + """Only load hooks when WAGTAILTRANS_SYNC_TREE is disabled""" + return + + if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + return + + if hasattr(page, 'canonical_page') and page.canonical_page: + return + + yield widgets.ButtonWithDropdownFromHook( + _("Translate into"), + hook_name='wagtailtrans_dropdown_hook', + page=page, + page_perms=page_perms, + is_parent=is_parent, + priority=10 + ) @hooks.register('wagtailtrans_dropdown_hook') def page_translations_menu_items(page, page_perms, is_parent=False): @@ -116,7 +117,7 @@ def edit_in_language_button(page, page_perms, is_parent=False): clear interface to work in. """ - if not hasattr(page, 'language'): + if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): return yield widgets.ButtonWithDropdownFromHook( diff --git a/tests/_sandbox/pages/migrations/0003_article.py b/tests/_sandbox/pages/migrations/0003_article.py new file mode 100644 index 00000000..1393af4c --- /dev/null +++ b/tests/_sandbox/pages/migrations/0003_article.py @@ -0,0 +1,31 @@ +# Generated by Django 2.1.5 on 2019-02-08 16:39 + +from django.db import migrations, models +import django.db.models.deletion +import wagtail.core.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtailcore', '0041_group_collection_permissions_verbose_name_plural'), + ('wagtailimages', '0001_squashed_0021'), + ('pages', '0002_auto_20161214_0842'), + ] + + operations = [ + migrations.CreateModel( + name='Article', + fields=[ + ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), + ('date', models.DateField()), + ('intro_text', wagtail.core.fields.RichTextField()), + ('image', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to='wagtailimages.Image')), + ('language', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='wagtailtrans.Language')), + ], + options={ + 'abstract': False, + }, + bases=('wagtailcore.page',), + ), + ] diff --git a/tests/_sandbox/pages/models.py b/tests/_sandbox/pages/models.py index bbbbcbd3..2cfc1ed8 100644 --- a/tests/_sandbox/pages/models.py +++ b/tests/_sandbox/pages/models.py @@ -22,3 +22,20 @@ class HomePage(TranslatablePage): ] subpage_types = ['HomePage'] + + +class Article(Page): + image = models.ForeignKey( + 'wagtailimages.Image', null=True, blank=True, on_delete=models.SET_NULL, related_name='+') + date = models.DateField() + language = models.ForeignKey('wagtailtrans.Language', on_delete=models.SET_NULL, null=True) + intro_text = RichTextField() + + content_panels = [ + FieldPanel('title', 'full title'), + FieldPanel('date'), + FieldPanel('intro_text'), + FieldPanel('language'), + ImageChooserPanel('image') + + ] diff --git a/tests/factories/pages.py b/tests/factories/pages.py index 64f351fe..7de4e7d1 100644 --- a/tests/factories/pages.py +++ b/tests/factories/pages.py @@ -1,11 +1,14 @@ +import datetime + import factory +from factory.fuzzy import FuzzyDate from wagtail.core.models import Page from wagtail.images.tests.utils import ( get_image_model, get_test_image_file) from wagtailtrans import models -from tests._sandbox.pages.models import HomePage +from tests._sandbox.pages.models import HomePage, Article from tests.factories import language @@ -74,3 +77,35 @@ class WagtailPageFactory(factory.DjangoModelFactory): class Meta: model = Page + + @classmethod + def _create(cls, *args, **kwargs): + model = args[0] + parent = kwargs.pop( + 'parent', + Page.objects.get(path='0001') + ) + + try: + return model.objects.get(title=kwargs['title']) + except model.DoesNotExist: + kwargs['depth'] = parent.depth + 1 + + if kwargs['depth'] == 0: + return model.add_root(**kwargs) + else: + return parent.add_child( + instance=model( + **kwargs + ) + ) + + +class ArticleFactory(WagtailPageFactory): + title = 'Article 1' + intro_text = 'This is an article' + date = FuzzyDate(datetime.date(2019, 1, 1)) + language = factory.SubFactory(language.LanguageFactory) + + class Meta: + model = Article diff --git a/tests/unit/test_wagtail_hooks.py b/tests/unit/test_wagtail_hooks.py new file mode 100644 index 00000000..e22a50c4 --- /dev/null +++ b/tests/unit/test_wagtail_hooks.py @@ -0,0 +1,32 @@ +import pytest +from django.test import override_settings + +from tests.factories.pages import ArticleFactory +from wagtailtrans.models import TranslatablePage +from wagtailtrans.wagtail_hooks import ( + edit_in_language_button, page_translations_menu) + + +@pytest.mark.django_db +class TestWagtailHooks: + def setup(self): + self.article = ArticleFactory() + self.transpage = TranslatablePage() + + def test_edit_in_language_wagtail_hook_with_regular_page(self): + assert list(edit_in_language_button(self.article, page_perms=[])) == [] + + def test_edit_in_language_wagtail_hook_translateable_page(self): + result = list(edit_in_language_button(self.transpage, page_perms=[])) + assert len(result) == 1 + assert result[0].label == 'Edit in' + + def test_page_translations_menu_with_regular_page(self): + with override_settings(WAGTAILTRANS_SYNC_TREE=False): + assert list(page_translations_menu(self.article, page_perms=[])) == [] + + def test_page_translations_menu_translateable_page(self): + with override_settings(WAGTAILTRANS_SYNC_TREE=False): + result = list(page_translations_menu(self.transpage, page_perms=[])) + assert len(result) == 1 + assert result[0].label == 'Translate into' From 0bc5ff51a655207399ba384b65a3bf9143ed9e62 Mon Sep 17 00:00:00 2001 From: Oktay Altay Date: Tue, 12 Feb 2019 09:41:15 +0100 Subject: [PATCH 2/5] remove redundent field and change the migration --- tests/_sandbox/pages/migrations/0003_article.py | 6 ++---- tests/_sandbox/pages/models.py | 4 ---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/_sandbox/pages/migrations/0003_article.py b/tests/_sandbox/pages/migrations/0003_article.py index 1393af4c..30bfccc2 100644 --- a/tests/_sandbox/pages/migrations/0003_article.py +++ b/tests/_sandbox/pages/migrations/0003_article.py @@ -1,4 +1,4 @@ -# Generated by Django 2.1.5 on 2019-02-08 16:39 +# Generated by Django 2.1.5 on 2019-02-12 08:23 from django.db import migrations, models import django.db.models.deletion @@ -8,8 +8,7 @@ class Migration(migrations.Migration): dependencies = [ - ('wagtailcore', '0041_group_collection_permissions_verbose_name_plural'), - ('wagtailimages', '0001_squashed_0021'), + ('wagtailcore', '0040_page_draft_title'), ('pages', '0002_auto_20161214_0842'), ] @@ -20,7 +19,6 @@ class Migration(migrations.Migration): ('page_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='wagtailcore.Page')), ('date', models.DateField()), ('intro_text', wagtail.core.fields.RichTextField()), - ('image', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='+', to='wagtailimages.Image')), ('language', models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='wagtailtrans.Language')), ], options={ diff --git a/tests/_sandbox/pages/models.py b/tests/_sandbox/pages/models.py index 2cfc1ed8..e9bf53ff 100644 --- a/tests/_sandbox/pages/models.py +++ b/tests/_sandbox/pages/models.py @@ -25,8 +25,6 @@ class HomePage(TranslatablePage): class Article(Page): - image = models.ForeignKey( - 'wagtailimages.Image', null=True, blank=True, on_delete=models.SET_NULL, related_name='+') date = models.DateField() language = models.ForeignKey('wagtailtrans.Language', on_delete=models.SET_NULL, null=True) intro_text = RichTextField() @@ -36,6 +34,4 @@ class Article(Page): FieldPanel('date'), FieldPanel('intro_text'), FieldPanel('language'), - ImageChooserPanel('image') - ] From 1d62740118c5a5d0239d8dffe364b500e604be8f Mon Sep 17 00:00:00 2001 From: Oktay Altay Date: Tue, 12 Feb 2019 10:21:58 +0100 Subject: [PATCH 3/5] update the tests --- src/wagtailtrans/wagtail_hooks.py | 40 +++++++++++++++---------------- tests/unit/test_wagtail_hooks.py | 36 +++++++++++++++++++++------- 2 files changed, 46 insertions(+), 30 deletions(-) diff --git a/src/wagtailtrans/wagtail_hooks.py b/src/wagtailtrans/wagtail_hooks.py index 14b7820b..46be8df8 100644 --- a/src/wagtailtrans/wagtail_hooks.py +++ b/src/wagtailtrans/wagtail_hooks.py @@ -41,27 +41,25 @@ def global_admin_js(): path=static('wagtailtrans/js/site_languages_editor.js')) ) - -@hooks.register('register_page_listing_buttons') -def page_translations_menu(page, page_perms, is_parent=False): - if get_wagtailtrans_setting('SYNC_TREE'): - """Only load hooks when WAGTAILTRANS_SYNC_TREE is disabled""" - return - - if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): - return - - if hasattr(page, 'canonical_page') and page.canonical_page: - return - - yield widgets.ButtonWithDropdownFromHook( - _("Translate into"), - hook_name='wagtailtrans_dropdown_hook', - page=page, - page_perms=page_perms, - is_parent=is_parent, - priority=10 - ) +if not get_wagtailtrans_setting('SYNC_TREE'): + """Only load hooks when WAGTAILTRANS_SYNC_TREE is disabled""" + + @hooks.register('register_page_listing_buttons') + def page_translations_menu(page, page_perms, is_parent=False): + if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + return + + if hasattr(page, 'canonical_page') and page.canonical_page: + return + + yield widgets.ButtonWithDropdownFromHook( + 'Translate into', + hook_name='wagtailtrans_dropdown_hook', + page=page, + page_perms=page_perms, + is_parent=is_parent, + priority=10 + ) @hooks.register('wagtailtrans_dropdown_hook') def page_translations_menu_items(page, page_perms, is_parent=False): diff --git a/tests/unit/test_wagtail_hooks.py b/tests/unit/test_wagtail_hooks.py index e22a50c4..30a3f66e 100644 --- a/tests/unit/test_wagtail_hooks.py +++ b/tests/unit/test_wagtail_hooks.py @@ -1,10 +1,11 @@ import pytest -from django.test import override_settings + +from wagtail.admin import widgets +from wagtail.core import hooks from tests.factories.pages import ArticleFactory from wagtailtrans.models import TranslatablePage -from wagtailtrans.wagtail_hooks import ( - edit_in_language_button, page_translations_menu) +from wagtailtrans.wagtail_hooks import edit_in_language_button @pytest.mark.django_db @@ -13,6 +14,25 @@ def setup(self): self.article = ArticleFactory() self.transpage = TranslatablePage() + @hooks.register('register_page_listing_buttons') + def page_translations_menu(self, page, page_perms, is_parent=False): + # To test this hook i need to add it here, if we don't do this + # we can't import the hook like the edit_in_language_button + if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + return + + if hasattr(page, 'canonical_page') and page.canonical_page: + return + + yield widgets.ButtonWithDropdownFromHook( + 'Translate into', + hook_name='wagtailtrans_dropdown_hook', + page=page, + page_perms=page_perms, + is_parent=is_parent, + priority=10 + ) + def test_edit_in_language_wagtail_hook_with_regular_page(self): assert list(edit_in_language_button(self.article, page_perms=[])) == [] @@ -22,11 +42,9 @@ def test_edit_in_language_wagtail_hook_translateable_page(self): assert result[0].label == 'Edit in' def test_page_translations_menu_with_regular_page(self): - with override_settings(WAGTAILTRANS_SYNC_TREE=False): - assert list(page_translations_menu(self.article, page_perms=[])) == [] + assert list(self.page_translations_menu(self.article, page_perms=[])) == [] def test_page_translations_menu_translateable_page(self): - with override_settings(WAGTAILTRANS_SYNC_TREE=False): - result = list(page_translations_menu(self.transpage, page_perms=[])) - assert len(result) == 1 - assert result[0].label == 'Translate into' + result = list(self.page_translations_menu(self.transpage, page_perms=[])) + assert len(result) == 1 + assert result[0].label == 'Translate into' From 4dfeda8920ec6ccbb4a2fe5a40f70ab560ca07cf Mon Sep 17 00:00:00 2001 From: Oktay Altay Date: Fri, 1 Mar 2019 09:16:00 +0100 Subject: [PATCH 4/5] remove redundant code if page is translatable then it will always have an Language --- src/wagtailtrans/wagtail_hooks.py | 4 ++-- tests/unit/test_wagtail_hooks.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wagtailtrans/wagtail_hooks.py b/src/wagtailtrans/wagtail_hooks.py index 46be8df8..5ec0f300 100644 --- a/src/wagtailtrans/wagtail_hooks.py +++ b/src/wagtailtrans/wagtail_hooks.py @@ -46,7 +46,7 @@ def global_admin_js(): @hooks.register('register_page_listing_buttons') def page_translations_menu(page, page_perms, is_parent=False): - if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + if not isinstance(page, TranslatablePage): return if hasattr(page, 'canonical_page') and page.canonical_page: @@ -115,7 +115,7 @@ def edit_in_language_button(page, page_perms, is_parent=False): clear interface to work in. """ - if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + if not isinstance(page, TranslatablePage): return yield widgets.ButtonWithDropdownFromHook( diff --git a/tests/unit/test_wagtail_hooks.py b/tests/unit/test_wagtail_hooks.py index 30a3f66e..0060fa7b 100644 --- a/tests/unit/test_wagtail_hooks.py +++ b/tests/unit/test_wagtail_hooks.py @@ -18,7 +18,7 @@ def setup(self): def page_translations_menu(self, page, page_perms, is_parent=False): # To test this hook i need to add it here, if we don't do this # we can't import the hook like the edit_in_language_button - if not isinstance(page, TranslatablePage) or not hasattr(page, 'language'): + if not isinstance(page, TranslatablePage): return if hasattr(page, 'canonical_page') and page.canonical_page: From cd968d7e0fc744d2b52bab90a7aacd4b9d437a0b Mon Sep 17 00:00:00 2001 From: Oktay Altay Date: Fri, 1 Mar 2019 17:24:55 +0100 Subject: [PATCH 5/5] make use of issubclass --- src/wagtailtrans/wagtail_hooks.py | 4 ++-- tests/unit/test_wagtail_hooks.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/wagtailtrans/wagtail_hooks.py b/src/wagtailtrans/wagtail_hooks.py index 5ec0f300..8a4de2d0 100644 --- a/src/wagtailtrans/wagtail_hooks.py +++ b/src/wagtailtrans/wagtail_hooks.py @@ -46,7 +46,7 @@ def global_admin_js(): @hooks.register('register_page_listing_buttons') def page_translations_menu(page, page_perms, is_parent=False): - if not isinstance(page, TranslatablePage): + if not issubclass(page.__class__, TranslatablePage): return if hasattr(page, 'canonical_page') and page.canonical_page: @@ -115,7 +115,7 @@ def edit_in_language_button(page, page_perms, is_parent=False): clear interface to work in. """ - if not isinstance(page, TranslatablePage): + if not issubclass(page.__class__, TranslatablePage): return yield widgets.ButtonWithDropdownFromHook( diff --git a/tests/unit/test_wagtail_hooks.py b/tests/unit/test_wagtail_hooks.py index 0060fa7b..6b9ce5ac 100644 --- a/tests/unit/test_wagtail_hooks.py +++ b/tests/unit/test_wagtail_hooks.py @@ -18,7 +18,7 @@ def setup(self): def page_translations_menu(self, page, page_perms, is_parent=False): # To test this hook i need to add it here, if we don't do this # we can't import the hook like the edit_in_language_button - if not isinstance(page, TranslatablePage): + if not issubclass(page.__class__, TranslatablePage): return if hasattr(page, 'canonical_page') and page.canonical_page: