From 6b9f019959e47221291ca1f4723f389e6c6cda93 Mon Sep 17 00:00:00 2001 From: Janet Yu Date: Fri, 13 Mar 2015 13:50:42 -0700 Subject: [PATCH] Fix addition of plugin panel to panel group Revert fix from bug #1329050, which adds a plugin panel to its dashboard's class. Adding a plugin panel to a dashboard whose class has its panels defined in a tuple will fail because the new plugin cannot be appended to the tuple. The code errors out before the panel gets added to the dashboard's panel list. However, at this point, the panel has already been registered with the dashboard. This causes Dashboard.get_panels() to see the panel in its registry but not in its panel groups. Consequently, the panel gets put in the "Other" panel group instead of its configured one, as described in bug #1378558. By not adding the the plugin panel to its dashboard's class, the panel gets added to its dashboard's panel list. Now the original issue from bug #1329050 resurfaces. The root cause of that is that every time Dashboard._autodiscover() is called, it instantiates all its panel group classes, wiping out any panels previously added to any of the panel groups. Avoid this by always processing all the plugin panel groups before any of the plugin panels, which also ensures that the panel groups exist before their panels. Making this change exposed an issue where the Horizon URLs were being loaded twice during the plugin infrastructure's unit tests, causing plugin panels to be added multiple times to their dashboards. Fix that by removing the second, unnecessary call. Also create a second test plugin panel group and panel to test the scenarios described in the two bugs addressed here, adding two panel groups at a time. Change-Id: Id6a99c3ff18102c8f47431638d4dda98f14ec641 Closes-Bug: 1378558 (cherry picked from commit e2ef492311a427fe3b5b41a6df6bd9be5a22c174) (cherry picked from commit 5c1604275bfbe621e5e168f8ba1f937d308d6ab8) --- horizon/base.py | 11 +++++-- openstack_dashboard/test/helpers.py | 3 -- .../test/test_panels/plugin_panel/views.py | 1 + .../test/test_panels/second_panel/__init__.py | 0 .../test/test_panels/second_panel/panel.py | 18 ++++++++++++ .../templates/second_panel/index.html | 11 +++++++ .../test/test_panels/second_panel/urls.py | 21 ++++++++++++++ .../test/test_panels/second_panel/views.py | 18 ++++++++++++ .../_30_admin_add_second_panel_group.py | 6 ++++ .../_40_admin_add_panel_to_second_group.py | 10 +++++++ .../test/test_plugins/panel_group_tests.py | 29 ++++++++++++++++++- .../test/test_plugins/panel_tests.py | 10 +++++++ 12 files changed, 131 insertions(+), 7 deletions(-) create mode 100644 openstack_dashboard/test/test_panels/second_panel/__init__.py create mode 100644 openstack_dashboard/test/test_panels/second_panel/panel.py create mode 100644 openstack_dashboard/test/test_panels/second_panel/templates/second_panel/index.html create mode 100644 openstack_dashboard/test/test_panels/second_panel/urls.py create mode 100644 openstack_dashboard/test/test_panels/second_panel/views.py create mode 100644 openstack_dashboard/test/test_plugins/panel_group_config/_30_admin_add_second_panel_group.py create mode 100644 openstack_dashboard/test/test_plugins/panel_group_config/_40_admin_add_panel_to_second_group.py diff --git a/horizon/base.py b/horizon/base.py index 17249c150e..738deb3458 100644 --- a/horizon/base.py +++ b/horizon/base.py @@ -847,14 +847,21 @@ class Site(Registry, HorizonComponent): """ panel_customization = self._conf.get("panel_customization", []) + # Process all the panel groups first so that they exist before panels + # are added to them and Dashboard._autodiscover() doesn't wipe out any + # panels previously added when its panel groups are instantiated. + panel_configs = [] for config in panel_customization: if config.get('PANEL'): - self._process_panel_configuration(config) + panel_configs.append(config) elif config.get('PANEL_GROUP'): self._process_panel_group_configuration(config) else: LOG.warning("Skipping %s because it doesn't have PANEL or " "PANEL_GROUP defined.", config.__name__) + # Now process the panels. + for config in panel_configs: + self._process_panel_configuration(config) def _process_panel_configuration(self, config): """Add, remove and set default panels on the dashboard.""" @@ -890,8 +897,6 @@ class Site(Registry, HorizonComponent): panel = getattr(mod, panel_cls) dashboard_cls.register(panel) if panel_group: - dashboard_cls.get_panel_group(panel_group).__class__.\ - panels.append(panel.slug) dashboard_cls.get_panel_group(panel_group).\ panels.append(panel.slug) else: diff --git a/openstack_dashboard/test/helpers.py b/openstack_dashboard/test/helpers.py index 11a9b46732..2102241106 100644 --- a/openstack_dashboard/test/helpers.py +++ b/openstack_dashboard/test/helpers.py @@ -454,9 +454,6 @@ class PluginTestCase(TestCase): self.old_horizon_config = conf.HORIZON_CONFIG conf.HORIZON_CONFIG = conf.LazySettings() base.Horizon._urls() - # Trigger discovery, registration, and URLconf generation if it - # hasn't happened yet. - self.client.get("/") # Store our original dashboards self._discovered_dashboards = base.Horizon._registry.keys() # Gather up and store our original panels for each dashboard diff --git a/openstack_dashboard/test/test_panels/plugin_panel/views.py b/openstack_dashboard/test/test_panels/plugin_panel/views.py index 8609a1a114..877bdc9694 100644 --- a/openstack_dashboard/test/test_panels/plugin_panel/views.py +++ b/openstack_dashboard/test/test_panels/plugin_panel/views.py @@ -15,3 +15,4 @@ from django.views.generic import TemplateView # noqa class IndexView(TemplateView): template_name = 'admin/plugin_panel/index.html' + page_title = 'Plugin-based Panel' diff --git a/openstack_dashboard/test/test_panels/second_panel/__init__.py b/openstack_dashboard/test/test_panels/second_panel/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openstack_dashboard/test/test_panels/second_panel/panel.py b/openstack_dashboard/test/test_panels/second_panel/panel.py new file mode 100644 index 0000000000..52f61b9818 --- /dev/null +++ b/openstack_dashboard/test/test_panels/second_panel/panel.py @@ -0,0 +1,18 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import horizon + + +class SecondPanel(horizon.Panel): + name = "Second Plugin Panel" + slug = 'second_panel' diff --git a/openstack_dashboard/test/test_panels/second_panel/templates/second_panel/index.html b/openstack_dashboard/test/test_panels/second_panel/templates/second_panel/index.html new file mode 100644 index 0000000000..62a1d6b496 --- /dev/null +++ b/openstack_dashboard/test/test_panels/second_panel/templates/second_panel/index.html @@ -0,0 +1,11 @@ +{% extends 'base.html' %} +{% load i18n %} +{% block title %}Second Plugin-based Panel{% endblock %} + +{% block main %} +
+
+ Second Plugin-based Panel +
+
+{% endblock %} diff --git a/openstack_dashboard/test/test_panels/second_panel/urls.py b/openstack_dashboard/test/test_panels/second_panel/urls.py new file mode 100644 index 0000000000..277a98229b --- /dev/null +++ b/openstack_dashboard/test/test_panels/second_panel/urls.py @@ -0,0 +1,21 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from django.conf.urls import patterns +from django.conf.urls import url + +from openstack_dashboard.test.test_panels.second_panel import views + +urlpatterns = patterns( + '', + url(r'^$', views.IndexView.as_view(), name='index'), +) diff --git a/openstack_dashboard/test/test_panels/second_panel/views.py b/openstack_dashboard/test/test_panels/second_panel/views.py new file mode 100644 index 0000000000..7b6f11c6b5 --- /dev/null +++ b/openstack_dashboard/test/test_panels/second_panel/views.py @@ -0,0 +1,18 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from django.views.generic import TemplateView # noqa + + +class IndexView(TemplateView): + template_name = 'admin/second_panel/index.html' + page_title = 'Second Plugin-based Panel' diff --git a/openstack_dashboard/test/test_plugins/panel_group_config/_30_admin_add_second_panel_group.py b/openstack_dashboard/test/test_plugins/panel_group_config/_30_admin_add_second_panel_group.py new file mode 100644 index 0000000000..407d19984b --- /dev/null +++ b/openstack_dashboard/test/test_plugins/panel_group_config/_30_admin_add_second_panel_group.py @@ -0,0 +1,6 @@ +# The name of the panel group to be added to HORIZON_CONFIG. Required. +PANEL_GROUP = 'second_panel_group' +# The display name of the PANEL_GROUP. Required. +PANEL_GROUP_NAME = 'Second Plugin Panel Group' +# The name of the dashboard the PANEL_GROUP associated with. Required. +PANEL_GROUP_DASHBOARD = 'admin' diff --git a/openstack_dashboard/test/test_plugins/panel_group_config/_40_admin_add_panel_to_second_group.py b/openstack_dashboard/test/test_plugins/panel_group_config/_40_admin_add_panel_to_second_group.py new file mode 100644 index 0000000000..7ce597584f --- /dev/null +++ b/openstack_dashboard/test/test_plugins/panel_group_config/_40_admin_add_panel_to_second_group.py @@ -0,0 +1,10 @@ +# The name of the panel to be added to HORIZON_CONFIG. Required. +PANEL = 'second_panel' +# The name of the dashboard the PANEL associated with. Required. +PANEL_DASHBOARD = 'admin' +# The name of the panel group the PANEL is associated with. +PANEL_GROUP = 'second_panel_group' + +# Python panel class of the PANEL to be added. +ADD_PANEL = \ + 'openstack_dashboard.test.test_panels.second_panel.panel.SecondPanel' diff --git a/openstack_dashboard/test/test_plugins/panel_group_tests.py b/openstack_dashboard/test/test_plugins/panel_group_tests.py index ff8f4e8054..0bb82d4345 100644 --- a/openstack_dashboard/test/test_plugins/panel_group_tests.py +++ b/openstack_dashboard/test/test_plugins/panel_group_tests.py @@ -20,11 +20,14 @@ import horizon from openstack_dashboard.test import helpers as test from openstack_dashboard.test.test_panels.plugin_panel \ import panel as plugin_panel +from openstack_dashboard.test.test_panels.second_panel \ + import panel as second_panel import openstack_dashboard.test.test_plugins.panel_group_config from openstack_dashboard.utils import settings as util_settings PANEL_GROUP_SLUG = 'plugin_panel_group' +SECOND_PANEL_GROUP_SLUG = 'second_panel_group' HORIZON_CONFIG = copy.deepcopy(settings.HORIZON_CONFIG) INSTALLED_APPS = list(settings.INSTALLED_APPS) @@ -40,7 +43,31 @@ class PanelGroupPluginTests(test.PluginTestCase): dashboard = horizon.get_dashboard("admin") self.assertIsNotNone(dashboard.get_panel_group(PANEL_GROUP_SLUG)) - def test_add_panel(self): + def test_add_second_panel_group(self): + # Check that the second panel group was added to the dashboard. dashboard = horizon.get_dashboard("admin") + self.assertIsNotNone( + dashboard.get_panel_group(SECOND_PANEL_GROUP_SLUG)) + + def test_add_panel(self): + # Check that the panel is in its configured dashboard and panel group. + dashboard = horizon.get_dashboard("admin") + panel_group = dashboard.get_panel_group(PANEL_GROUP_SLUG) self.assertIn(plugin_panel.PluginPanel, [p.__class__ for p in dashboard.get_panels()]) + self.assertIn(plugin_panel.PluginPanel, + [p.__class__ for p in panel_group]) + + def test_add_second_panel(self): + # Check that the second panel is in its configured dashboard and panel + # group. + dashboard = horizon.get_dashboard("admin") + second_panel_group = dashboard.get_panel_group(SECOND_PANEL_GROUP_SLUG) + self.assertIn(second_panel.SecondPanel, + [p.__class__ for p in dashboard.get_panels()]) + self.assertIn(second_panel.SecondPanel, + [p.__class__ for p in second_panel_group]) + + def test_unregistered_panel_group(self): + dashboard = horizon.get_dashboard("admin") + self.assertNotIn("nonexistent_panel", dashboard.get_panel_groups()) diff --git a/openstack_dashboard/test/test_plugins/panel_tests.py b/openstack_dashboard/test/test_plugins/panel_tests.py index 7f8c6b4a69..c3e50a9891 100644 --- a/openstack_dashboard/test/test_plugins/panel_tests.py +++ b/openstack_dashboard/test/test_plugins/panel_tests.py @@ -38,13 +38,23 @@ util_settings.update_dashboards([ class PanelPluginTests(test.PluginTestCase): def test_add_panel(self): dashboard = horizon.get_dashboard("admin") + panel_group = dashboard.get_panel_group('admin') + # Check that the panel is in its configured dashboard. self.assertIn(plugin_panel.PluginPanel, [p.__class__ for p in dashboard.get_panels()]) + # Check that the panel is in its configured panel group. + self.assertIn(plugin_panel.PluginPanel, + [p.__class__ for p in panel_group]) def test_remove_panel(self): dashboard = horizon.get_dashboard("admin") + panel_group = dashboard.get_panel_group('admin') + # Check that the panel is no longer in the configured dashboard. self.assertNotIn(info_panel.Info, [p.__class__ for p in dashboard.get_panels()]) + # Check that the panel is no longer in the configured panel group. + self.assertNotIn(info_panel.Info, + [p.__class__ for p in panel_group]) def test_default_panel(self): dashboard = horizon.get_dashboard("admin")