From 2044b7e9306aaceb9b3d61ecdde4e258f7af2c94 Mon Sep 17 00:00:00 2001 From: "LB (Ben Johnston)" Date: Thu, 21 Feb 2019 22:36:31 +1000 Subject: [PATCH] Add panel configuration checks (#5093) * checks for Page InlinePanel related model panels * add panels check to modelAdmin models * add and revise tests for panel model checks * revise related model usage * remove unused keys from format string * fix up unique error collection * rework check_panels_in_model - variable names and string formatting * rework tests for check_panels_in_model to use new string formatting plus linting of some unused imports * add checks to snippet models * use consistent naming for returning errors from checks in modeladmin * add tests for snippets check_panels_in_model checks * ignore vscode config files * remove additional line added --- .gitignore | 3 + wagtail/admin/checks.py | 80 +++++++++++++++++ wagtail/admin/tests/test_edit_handlers.py | 89 ++++++++++++++++++- wagtail/contrib/modeladmin/options.py | 20 +++++ .../tests/test_simple_modeladmin.py | 88 ++++++++++++++++++ wagtail/snippets/models.py | 8 ++ wagtail/snippets/tests.py | 38 ++++++++ 7 files changed, 323 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 91fe3857f..6879e6fe9 100644 --- a/.gitignore +++ b/.gitignore @@ -23,3 +23,6 @@ npm-debug.log* *.iws coverage/ client/node_modules + +### vscode +.vscode diff --git a/wagtail/admin/checks.py b/wagtail/admin/checks.py index b08c112a8..5fd1c65bd 100644 --- a/wagtail/admin/checks.py +++ b/wagtail/admin/checks.py @@ -70,3 +70,83 @@ def get_form_class_check(app_configs, **kwargs): id='wagtailadmin.E002')) return errors + + +@register('panels') +def inline_panel_model_panels_check(app_configs, **kwargs): + from wagtail.core.models import get_page_models + + errors = [] + page_models = get_page_models() + + for cls in page_models: + errors.extend(check_panels_in_model(cls)) + + # filter out duplicate errors found for the same model + unique_errors = [] + for error in errors: + if error.msg not in [e.msg for e in unique_errors]: + unique_errors.append(error) + return unique_errors + + +def check_panels_in_model(cls, context='model'): + """Check panels configuration uses `panels` when `edit_handler` not in use.""" + from wagtail.core.models import Page + from wagtail.admin.edit_handlers import InlinePanel + + errors = [] + + if hasattr(cls, 'get_edit_handler'): + # must check the InlinePanel related models + edit_handler = cls.get_edit_handler() + for tab in edit_handler.children: + inline_panel_children = [ + panel for panel in tab.children if isinstance(panel, InlinePanel)] + for inline_panel_child in inline_panel_children: + errors.extend(check_panels_in_model( + inline_panel_child.db_field.related_model, + context='InlinePanel model', + )) + + if issubclass(cls, Page) or hasattr(cls, 'edit_handler'): + # Pages do not need to be checked for standalone tabbed_panel usage + # if edit_handler is used on any model, assume config is correct + return errors + + tabbed_panels = [ + 'content_panels', + 'promote_panels', + 'settings_panels', + ] + + for panel_name in tabbed_panels: + class_name = cls.__name__ + if not hasattr(cls, panel_name): + continue + + panel_name_short = panel_name.replace('_panels', '').title() + error_title = "{}.{} will have no effect on {} editing".format( + class_name, panel_name, context) + + if 'InlinePanel' in context: + error_hint = """Ensure that {} uses `panels` instead of `{}`. +There are no tabs on non-Page model editing within InlinePanels.""".format( + class_name, panel_name) + else: + error_hint = """Ensure that {} uses `panels` instead of `{}`\ +or set up an `edit_handler` if you want a tabbed editing interface. +There are no default tabs on non-Page models so there will be no \ +{} tab for the {} to render in.""".format( + class_name, panel_name, panel_name_short, panel_name) + + error = Warning( + error_title, + hint=error_hint, + obj=cls, + id='wagtailadmin.W002' + ) + + errors.append(error) + + return errors diff --git a/wagtail/admin/tests/test_edit_handlers.py b/wagtail/admin/tests/test_edit_handlers.py index 1348eca13..fbf63eb07 100644 --- a/wagtail/admin/tests/test_edit_handlers.py +++ b/wagtail/admin/tests/test_edit_handlers.py @@ -8,8 +8,9 @@ from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured from django.test import RequestFactory, TestCase, override_settings from wagtail.admin.edit_handlers import ( - FieldPanel, FieldRowPanel, InlinePanel, ObjectList, PageChooserPanel, RichTextFieldPanel, - TabbedInterface, extract_panel_definitions_from_model_class, get_form_for_model) + FieldPanel, FieldRowPanel, InlinePanel, ObjectList, PageChooserPanel, + RichTextFieldPanel, TabbedInterface, extract_panel_definitions_from_model_class, + get_form_for_model) from wagtail.admin.forms import WagtailAdminModelForm, WagtailAdminPageForm from wagtail.admin.rich_text import DraftailRichTextArea from wagtail.admin.widgets import AdminAutoHeightTextInput, AdminDateInput, AdminPageChooser @@ -17,7 +18,8 @@ from wagtail.core.models import Page, Site from wagtail.images.edit_handlers import ImageChooserPanel from wagtail.tests.testapp.forms import ValidatedPageForm from wagtail.tests.testapp.models import ( - EventPage, EventPageChooserModel, EventPageSpeaker, PageChooserModel, SimplePage, ValidatedPage) + EventPage, EventPageChooserModel, EventPageSpeaker, PageChooserModel, + SimplePage, ValidatedPage) from wagtail.tests.utils import WagtailTestUtils @@ -890,3 +892,84 @@ class TestInlinePanel(TestCase, WagtailTestUtils): with self.ignore_deprecation_warnings(): self.assertRaises(TypeError, lambda: InlinePanel(label="Speakers")) self.assertRaises(TypeError, lambda: InlinePanel(EventPage, 'speakers', label="Speakers", bacon="chunky")) + + +class TestInlinePanelRelatedModelPanelConfigChecks(TestCase): + + def setUp(self): + self.original_panels = EventPageSpeaker.panels + delattr(EventPageSpeaker, 'panels') + + def get_checks_result(): + # run checks only with the 'panels' tag + checks_result = checks.run_checks(tags=['panels']) + return [warning for warning in checks_result if warning.obj == EventPageSpeaker] + + self.warning_id = 'wagtailadmin.W002' + self.get_checks_result = get_checks_result + + def tearDown(self): + EventPageSpeaker.panels = self.original_panels + + def test_page_with_inline_model_with_tabbed_panel_only(self): + """Test that checks will warn against setting single tabbed panel on InlinePanel model""" + + EventPageSpeaker.settings_panels = [FieldPanel('first_name'), FieldPanel('last_name')] + + warning = checks.Warning( + "EventPageSpeaker.settings_panels will have no effect on InlinePanel model editing", + hint="""Ensure that EventPageSpeaker uses `panels` instead of `settings_panels`. +There are no tabs on non-Page model editing within InlinePanels.""", + obj=EventPageSpeaker, + id=self.warning_id, + ) + + checks_results = self.get_checks_result() + + self.assertIn(warning, checks_results) + + delattr(EventPageSpeaker, 'settings_panels') + + def test_page_with_inline_model_with_two_tabbed_panels(self): + """Test that checks will warn against multiple tabbed panels on InlinePanel models""" + + EventPageSpeaker.content_panels = [FieldPanel('first_name')] + EventPageSpeaker.promote_panels = [FieldPanel('last_name')] + + warning_1 = checks.Warning( + "EventPageSpeaker.content_panels will have no effect on InlinePanel model editing", + hint="""Ensure that EventPageSpeaker uses `panels` instead of `content_panels`. +There are no tabs on non-Page model editing within InlinePanels.""", + obj=EventPageSpeaker, + id=self.warning_id, + ) + warning_2 = checks.Warning( + "EventPageSpeaker.promote_panels will have no effect on InlinePanel model editing", + hint="""Ensure that EventPageSpeaker uses `panels` instead of `promote_panels`. +There are no tabs on non-Page model editing within InlinePanels.""", + obj=EventPageSpeaker, + id=self.warning_id, + ) + + checks_results = self.get_checks_result() + + self.assertIn(warning_1, checks_results) + self.assertIn(warning_2, checks_results) + + delattr(EventPageSpeaker, 'content_panels') + delattr(EventPageSpeaker, 'promote_panels') + + def test_page_with_inline_model_with_edit_handler(self): + """Checks should NOT warn if InlinePanel models use tabbed panels AND edit_handler""" + + EventPageSpeaker.content_panels = [FieldPanel('first_name')] + EventPageSpeaker.edit_handler = TabbedInterface( + ObjectList([FieldPanel('last_name')], heading='test') + ) + + # should not be any errors + self.assertEqual(self.get_checks_result(), []) + + # clean up for future checks + delattr(EventPageSpeaker, 'edit_handler') + delattr(EventPageSpeaker, 'content_panels') diff --git a/wagtail/contrib/modeladmin/options.py b/wagtail/contrib/modeladmin/options.py index 6aac69174..bf82ced59 100644 --- a/wagtail/contrib/modeladmin/options.py +++ b/wagtail/contrib/modeladmin/options.py @@ -1,9 +1,11 @@ from django.conf.urls import url from django.contrib.auth.models import Permission +from django.core import checks from django.core.exceptions import ImproperlyConfigured from django.db.models import Model from django.utils.safestring import mark_safe +from wagtail.admin.checks import check_panels_in_model from wagtail.core import hooks from wagtail.core.models import Page @@ -497,6 +499,14 @@ class ModelAdmin(WagtailRegisterable): queryset = queryset.not_type(self.model) return queryset + def register_with_wagtail(self): + super().register_with_wagtail() + + @checks.register('panels') + def modeladmin_model_check(app_configs, **kwargs): + errors = check_panels_in_model(self.model, 'modeladmin') + return errors + class ModelAdminGroup(WagtailRegisterable): """ @@ -584,6 +594,16 @@ class ModelAdminGroup(WagtailRegisterable): parent_page, queryset, request) return queryset + def register_with_wagtail(self): + super().register_with_wagtail() + + @checks.register('panels') + def modeladmin_model_check(app_configs, **kwargs): + errors = [] + for modeladmin_class in self.items: + errors.extend(check_panels_in_model(modeladmin_class.model)) + return errors + def modeladmin_register(modeladmin_class): """ diff --git a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py index 7a739eff0..6b753c453 100644 --- a/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py +++ b/wagtail/contrib/modeladmin/tests/test_simple_modeladmin.py @@ -2,8 +2,10 @@ from unittest import mock from django.contrib.auth import get_user_model from django.contrib.auth.models import Group +from django.core import checks from django.test import TestCase +from wagtail.admin.edit_handlers import FieldPanel, TabbedInterface from wagtail.images.models import Image from wagtail.images.tests.utils import get_test_image_file from wagtail.tests.modeladmintest.models import Author, Book, Publisher, Token @@ -515,3 +517,89 @@ class TestHeaderBreadcrumbs(TestCase, WagtailTestUtils): position_of_header_close = content_str.index('') position_of_breadcrumbs = content_str.index('