From e02c442c86a0e961442d2825d451ef0508acbd72 Mon Sep 17 00:00:00 2001 From: Gabriel Hurley Date: Sat, 24 Mar 2012 16:11:19 -0700 Subject: [PATCH] Adds support for tabs + tables. Creates new TableTab and TabbedTableView classes to support the complex logic involved in processing both table and tab actions in a single view. Fixes bug 964214. Change-Id: I3f70d77975593773bf783d31de06d2b724aad2d5 --- docs/source/ref/tabs.rst | 8 ++ .../instances_and_volumes/instances/tables.py | 3 +- .../instances_and_volumes/volumes/tables.py | 3 +- horizon/tables/__init__.py | 2 +- horizon/tables/base.py | 82 +++++++++----- horizon/tables/views.py | 49 ++++---- horizon/tabs/__init__.py | 4 +- horizon/tabs/base.py | 107 ++++++++++++++++-- horizon/tabs/views.py | 107 ++++++++++++++++-- .../horizon/common/_detail_table.html | 1 + horizon/tests/tabs_tests.py | 75 ++++++++++++ horizon/tests/templates/tab_group.html | 1 + 12 files changed, 367 insertions(+), 75 deletions(-) create mode 100644 horizon/templates/horizon/common/_detail_table.html create mode 100644 horizon/tests/templates/tab_group.html diff --git a/docs/source/ref/tabs.rst b/docs/source/ref/tabs.rst index 9da824ab27..807385caf2 100644 --- a/docs/source/ref/tabs.rst +++ b/docs/source/ref/tabs.rst @@ -27,6 +27,11 @@ view of data. .. autoclass:: Tab :members: +.. autoclass:: TableTab + :members: + + + TabView ======= @@ -35,3 +40,6 @@ the display of a :class:`~horizon.tabs.TabGroup` class. .. autoclass:: TabView :members: + +.. autoclass:: TabbedTableView + :members: diff --git a/horizon/dashboards/nova/instances_and_volumes/instances/tables.py b/horizon/dashboards/nova/instances_and_volumes/instances/tables.py index d178249c5f..79813fb1af 100644 --- a/horizon/dashboards/nova/instances_and_volumes/instances/tables.py +++ b/horizon/dashboards/nova/instances_and_volumes/instances/tables.py @@ -175,8 +175,7 @@ class LogLink(tables.LinkAction): class UpdateRow(tables.Row): ajax = True - @classmethod - def get_data(cls, request, instance_id): + def get_data(self, request, instance_id): instance = api.server_get(request, instance_id) flavors = api.flavor_list(request) keyed_flavors = [(str(flavor.id), flavor) for flavor in flavors] diff --git a/horizon/dashboards/nova/instances_and_volumes/volumes/tables.py b/horizon/dashboards/nova/instances_and_volumes/volumes/tables.py index ed4618910c..247a813de4 100644 --- a/horizon/dashboards/nova/instances_and_volumes/volumes/tables.py +++ b/horizon/dashboards/nova/instances_and_volumes/volumes/tables.py @@ -74,8 +74,7 @@ class CreateSnapshot(tables.LinkAction): class UpdateRow(tables.Row): ajax = True - @classmethod - def get_data(cls, request, volume_id): + def get_data(self, request, volume_id): volume = api.volume_get(request, volume_id) return volume diff --git a/horizon/tables/__init__.py b/horizon/tables/__init__.py index 1e40929de9..ef8060430f 100644 --- a/horizon/tables/__init__.py +++ b/horizon/tables/__init__.py @@ -18,4 +18,4 @@ from .actions import (Action, BatchAction, DeleteAction, LinkAction, FilterAction) from .base import DataTable, Column, Row -from .views import DataTableView, MultiTableView +from .views import DataTableView, MultiTableView, MultiTableMixin diff --git a/horizon/tables/base.py b/horizon/tables/base.py index d433b409ba..32cb4ef6df 100644 --- a/horizon/tables/base.py +++ b/horizon/tables/base.py @@ -306,21 +306,36 @@ class Row(html.HTMLElement): ajax = False ajax_action_name = "row_update" - def __init__(self, table, datum): + def __init__(self, table, datum=None): super(Row, self).__init__() self.table = table self.datum = datum - id_vals = {"table": self.table.name, - "sep": STRING_SEPARATOR, - "id": table.get_object_id(datum)} - self.id = "%(table)s%(sep)srow%(sep)s%(id)s" % id_vals - if self.ajax: - interval = settings.HORIZON_CONFIG.get('ajax_poll_interval', 2500) - self.attrs['data-update-interval'] = interval - self.attrs['data-update-url'] = self.get_ajax_update_url() - self.classes.append("ajax-update") + if self.datum: + self.load_cells() + else: + self.id = None + self.cells = [] + def load_cells(self, datum=None): + """ + Load the row's data (either provided at initialization or as an + argument to this function), initiailize all the cells contained + by this row, and set the appropriate row properties which require + the row's data to be determined. + + This function is called automatically by + :meth:`~horizon.tables.Row.__init__` if the ``datum`` argument is + provided. However, by not providing the data during initialization + this function allows for the possibility of a two-step loading + pattern when you need a row instance but don't yet have the data + available. + """ # Compile all the cells on instantiation. + table = self.table + if datum: + self.datum = datum + else: + datum = self.datum cells = [] for column in table.columns.values(): if column.auto == "multi_select": @@ -338,8 +353,18 @@ class Row(html.HTMLElement): cells.append((column.name or column.auto, cell)) self.cells = SortedDict(cells) + if self.ajax: + interval = settings.HORIZON_CONFIG.get('ajax_poll_interval', 2500) + self.attrs['data-update-interval'] = interval + self.attrs['data-update-url'] = self.get_ajax_update_url() + self.classes.append("ajax-update") + # Add the row's status class and id to the attributes to be rendered. self.classes.append(self.status_class) + id_vals = {"table": self.table.name, + "sep": STRING_SEPARATOR, + "id": table.get_object_id(datum)} + self.id = "%(table)s%(sep)srow%(sep)s%(id)s" % id_vals self.attrs['id'] = self.id def __repr__(self): @@ -379,14 +404,13 @@ class Row(html.HTMLElement): "obj_id": self.table.get_object_id(self.datum)}) return "%s?%s" % (table_url, params) - @classmethod - def get_data(cls, request, obj_id): + def get_data(self, request, obj_id): """ Fetches the updated data for the row based on the object id passed in. Must be implemented by a subclass to allow AJAX updating. """ raise NotImplementedError("You must define a get_data method on %s" - % cls.__name__) + % self.__class__.__name__) class Cell(html.HTMLElement): @@ -756,7 +780,7 @@ class DataTable(object): For convenience it defaults to the value of ``request.get_full_path()`` with any query string stripped off, - e.g. the path at which the table was requested. + e.g. the path at which the table was requested. """ return self._meta.request.get_full_path().partition('?')[0] @@ -833,7 +857,8 @@ class DataTable(object): context = template.RequestContext(self._meta.request, extra_context) return row_actions_template.render(context) - def parse_action(self, action_string): + @staticmethod + def parse_action(action_string): """ Parses the ``action`` parameter (a string) sent back with the POST data. By default this parses a string formatted as @@ -885,12 +910,11 @@ class DataTable(object): _("Please select a row before taking that action.")) return None - def _check_handler(self): + @classmethod + def check_handler(cls, request): """ Determine whether the request should be handled by this table. """ - request = self._meta.request - if request.method == "POST" and "action" in request.POST: - table, action, obj_id = self.parse_action(request.POST["action"]) + table, action, obj_id = cls.parse_action(request.POST["action"]) elif "table" in request.GET and "action" in request.GET: table = request.GET["table"] action = request.GET["action"] @@ -904,22 +928,23 @@ class DataTable(object): Determine whether the request should be handled by a preemptive action on this table or by an AJAX row update before loading any data. """ - table_name, action_name, obj_id = self._check_handler() + request = self._meta.request + table_name, action_name, obj_id = self.check_handler(request) if table_name == self.name: # Handle AJAX row updating. - row_class = self._meta.row_class - if row_class.ajax and row_class.ajax_action_name == action_name: + new_row = self._meta.row_class(self) + if new_row.ajax and new_row.ajax_action_name == action_name: try: - datum = row_class.get_data(self._meta.request, obj_id) + datum = new_row.get_data(request, obj_id) + new_row.load_cells(datum) error = False except: datum = None - error = exceptions.handle(self._meta.request, ignore=True) - if self._meta.request.is_ajax(): + error = exceptions.handle(request, ignore=True) + if request.is_ajax(): if not error: - row = row_class(self, datum) - return HttpResponse(row.render()) + return HttpResponse(new_row.render()) else: return HttpResponse(status=error.status_code) @@ -938,7 +963,8 @@ class DataTable(object): Determine whether the request should be handled by any action on this table after data has been loaded. """ - table_name, action_name, obj_id = self._check_handler() + request = self._meta.request + table_name, action_name, obj_id = self.check_handler(request) if table_name == self.name and action_name: return self.take_action(action_name, obj_id) return None diff --git a/horizon/tables/views.py b/horizon/tables/views.py index 66e49f7a13..8101181054 100644 --- a/horizon/tables/views.py +++ b/horizon/tables/views.py @@ -17,20 +17,10 @@ from django.views import generic -class MultiTableView(generic.TemplateView): - """ - A class-based generic view to handle the display and processing of - multiple :class:`~horizon.tables.DataTable` classes in a single view. - - Three steps are required to use this view: set the ``table_classes`` - attribute with a tuple of the desired - :class:`~horizon.tables.DataTable` classes; - define a ``get_{{ table_name }}_data`` method for each table class - which returns a set of data for that table; and specify a template for - the ``template_name`` attribute. - """ +class MultiTableMixin(object): + """ A generic mixin which provides methods for handling DataTables. """ def __init__(self, *args, **kwargs): - super(MultiTableView, self).__init__(*args, **kwargs) + super(MultiTableMixin, self).__init__(*args, **kwargs) self.table_classes = getattr(self, "table_classes", []) self._data = {} self._tables = {} @@ -64,18 +54,36 @@ class MultiTableView(generic.TemplateView): return self._tables def get_context_data(self, **kwargs): - context = super(MultiTableView, self).get_context_data(**kwargs) + context = super(MultiTableMixin, self).get_context_data(**kwargs) tables = self.get_tables() for name, table in tables.items(): - if table.data is None: - raise AttributeError('%s has no data associated with it.' - % table.__class__.__name__) context["%s_table" % name] = table return context def has_more_data(self, table): return False + def handle_table(self, table): + name = table.name + data = self._get_data_dict() + self._tables[name].data = data[table._meta.name] + self._tables[name]._meta.has_more_data = self.has_more_data(table) + handled = self._tables[name].maybe_handle() + return handled + + +class MultiTableView(MultiTableMixin, generic.TemplateView): + """ + A class-based generic view to handle the display and processing of + multiple :class:`~horizon.tables.DataTable` classes in a single view. + + Three steps are required to use this view: set the ``table_classes`` + attribute with a tuple of the desired + :class:`~horizon.tables.DataTable` classes; + define a ``get_{{ table_name }}_data`` method for each table class + which returns a set of data for that table; and specify a template for + the ``template_name`` attribute. + """ def construct_tables(self): tables = self.get_tables().values() # Early out before data is loaded @@ -84,14 +92,11 @@ class MultiTableView(generic.TemplateView): if preempted: return preempted # Load data into each table and check for action handlers - data = self._get_data_dict() for table in tables: - name = table.name - self._tables[name].data = data[table._meta.name] - self._tables[name]._meta.has_more_data = self.has_more_data(table) - handled = self._tables[name].maybe_handle() + handled = self.handle_table(table) if handled: return handled + # If we didn't already return a response, returning None continues # with the view as normal. return None diff --git a/horizon/tabs/__init__.py b/horizon/tabs/__init__.py index de62cf8ac5..8f9834f42e 100644 --- a/horizon/tabs/__init__.py +++ b/horizon/tabs/__init__.py @@ -14,5 +14,5 @@ # License for the specific language governing permissions and limitations # under the License. -from .base import TabGroup, Tab -from .views import TabView +from .base import TabGroup, Tab, TableTab +from .views import TabView, TabbedTableView diff --git a/horizon/tabs/base.py b/horizon/tabs/base.py index 12ade19868..520b62da7b 100644 --- a/horizon/tabs/base.py +++ b/horizon/tabs/base.py @@ -95,14 +95,18 @@ class TabGroup(html.HTMLElement): self._tabs = SortedDict(tab_instances) if not self._set_active_tab(): self.tabs_not_available() - # Preload all data that will be loaded to allow errors to be displayed - for tab in self._tabs.values(): - if tab.load: - tab._context_data = tab.get_context_data(request) def __repr__(self): return "<%s: %s>" % (self.__class__.__name__, self.slug) + def load_tab_data(self): + """ + Preload all data that for the tabs that will be displayed. + """ + for tab in self._tabs.values(): + if tab.load and not tab.data_loaded: + tab._data = tab.get_context_data(self.request) + def get_id(self): """ Returns the id for this tab group. Defaults to the value of the tab @@ -171,6 +175,9 @@ class TabGroup(html.HTMLElement): return tab return None + def get_loaded_tabs(self): + return filter(lambda t: self.get_tab(t.slug), self._tabs.values()) + def get_selected_tab(self): """ Returns the tab specific by the GET request parameter. @@ -254,15 +261,20 @@ class Tab(html.HTMLElement): return load_preloaded and self._allowed and self._enabled @property - def context_data(self): - if not getattr(self, "_context_data", None): - self._context_data = self.get_context_data(self.request) - return self._context_data + def data(self): + if not getattr(self, "_data", None): + self._data = self.get_context_data(self.request) + return self._data + + @property + def data_loaded(self): + return getattr(self, "_data", None) is not None def render(self): """ - Renders the tab to HTML using the :meth:`~horizon.tabs.Tab.get_data` - method and the :meth:`~horizon.tabs.Tab.get_template_name` method. + Renders the tab to HTML using the + :meth:`~horizon.tabs.Tab.get_context_data` method and + the :meth:`~horizon.tabs.Tab.get_template_name` method. If :attr:`~horizon.tabs.Tab.preload` is ``False`` and ``force_load`` is not ``True``, or @@ -273,7 +285,7 @@ class Tab(html.HTMLElement): if not self.load: return '' try: - context = self.context_data + context = self.data except exceptions.Http302: raise except: @@ -350,3 +362,76 @@ class Tab(html.HTMLElement): The default behavior is to return ``True`` for all cases. """ return True + + +class TableTab(Tab): + """ + A :class:`~horizon.tabs.Tab` class which knows how to deal with + :class:`~horizon.tables.DataTable` classes rendered inside of it. + + This distinct class is required due to the complexity involved in handling + both dynamic tab loading, dynamic table updating and table actions all + within one view. + + .. attribute:: table_classes + + An iterable containing the :class:`~horizon.tables.DataTable` classes + which this tab will contain. Equivalent to the + :attr:`~horizon.tables.MultiTableView.table_classes` attribute on + :class:`~horizon.tables.MultiTableView`. For each table class you + need to define a corresponding ``get_{{ table_name }}_data`` method + as with :class:`~horizon.tables.MultiTableView`. + """ + table_classes = None + + def __init__(self, tab_group, request): + super(TableTab, self).__init__(tab_group, request) + if not self.table_classes: + class_name = self.__class__.__name__ + raise NotImplementedError("You must define a table_class " + "attribute on %s" % class_name) + # Instantiate our table classes but don't assign data yet + table_instances = [(table._meta.name, + table(request, **tab_group.kwargs)) + for table in self.table_classes] + self._tables = SortedDict(table_instances) + self._table_data_loaded = False + + def load_table_data(self): + """ + Calls the ``get_{{ table_name }}_data`` methods for each table class + and sets the data on the tables. + """ + # We only want the data to be loaded once, so we track if we have... + if not self._table_data_loaded: + for table_name, table in self._tables.items(): + # Fetch the data function. + func_name = "get_%s_data" % table_name + data_func = getattr(self, func_name, None) + if data_func is None: + cls_name = self.__class__.__name__ + raise NotImplementedError("You must define a %s method " + "on %s." % (func_name, cls_name)) + # Load the data. + table.data = data_func() + # Mark our data as loaded so we don't run the loaders again. + self._table_data_loaded = True + + def get_context_data(self, request): + """ + Adds a ``{{ table_name }}_table`` item to the context for each table + in the :attr:`~horizon.tabs.TableTab.table_classes` attribute. + + If only one table class is provided, a shortcut ``table`` context + variable is also added containing the single table. + """ + context = {} + # If the data hasn't been manually loaded before now, + # make certain it's loaded before setting the context. + self.load_table_data() + for table_name, table in self._tables.items(): + # If there's only one table class, add a shortcut name as well. + if len(self.table_classes) == 1: + context["table"] = table + context["%s_table" % table_name] = table + return context diff --git a/horizon/tabs/views.py b/horizon/tabs/views.py index b13a925e21..2a4addcea6 100644 --- a/horizon/tabs/views.py +++ b/horizon/tabs/views.py @@ -2,6 +2,8 @@ from django import http from django.views import generic from horizon import exceptions +from horizon import tables +from .base import TableTab class TabView(generic.TemplateView): @@ -17,30 +19,45 @@ class TabView(generic.TemplateView): inherits from :class:`horizon.tabs.TabGroup`. """ tab_group_class = None + _tab_group = None def __init__(self): if not self.tab_group_class: raise AttributeError("You must set the tab_group_class attribute " "on %s." % self.__class__.__name__) - def get_tabs(self, request, *args, **kwargs): - return self.tab_group_class(request, **kwargs) + def get_tabs(self, request, **kwargs): + """ Returns the initialized tab group for this view. """ + if self._tab_group is None: + self._tab_group = self.tab_group_class(request, **kwargs) + return self._tab_group - def get(self, request, *args, **kwargs): - context = self.get_context_data(**kwargs) + def get_context_data(self, **kwargs): + """ Adds the ``tab_group`` variable to the context data. """ + context = super(TabView, self).get_context_data(**kwargs) try: - tab_group = self.get_tabs(request, *args, **kwargs) + tab_group = self.get_tabs(self.request, **kwargs) context["tab_group"] = tab_group except: - exceptions.handle(request) + exceptions.handle(self.request) + return context - if request.is_ajax(): + def handle_tabbed_response(self, tab_group, context): + """ + Sends back an AJAX-appropriate response for the tab group if + required, otherwise renders the response as normal. + """ + if self.request.is_ajax(): if tab_group.selected: return http.HttpResponse(tab_group.selected.render()) else: return http.HttpResponse(tab_group.render()) return self.render_to_response(context) + def get(self, request, *args, **kwargs): + context = self.get_context_data(**kwargs) + return self.handle_tabbed_response(context["tab_group"], context) + def render_to_response(self, *args, **kwargs): response = super(TabView, self).render_to_response(*args, **kwargs) # Because Django's TemplateView uses the TemplateResponse class @@ -50,3 +67,79 @@ class TabView(generic.TemplateView): # of the exception-handling middleware. response.render() return response + + +class TabbedTableView(tables.MultiTableMixin, TabView): + def __init__(self, *args, **kwargs): + super(TabbedTableView, self).__init__(*args, **kwargs) + self.table_classes = [] + self._table_dict = {} + + def load_tabs(self): + """ + Loads the tab group, and compiles the table instances for each + table attached to any :class:`horizon.tabs.TableTab` instances on + the tab group. This step is necessary before processing any + tab or table actions. + """ + tab_group = self.get_tabs(self.request, **self.kwargs) + tabs = tab_group.get_tabs() + for tab in [t for t in tabs if issubclass(t.__class__, TableTab)]: + self.table_classes.extend(tab.table_classes) + for table in tab._tables.values(): + self._table_dict[table._meta.name] = {'table': table, + 'tab': tab} + + def get_tables(self): + """ A no-op on this class. Tables are handled at the tab level. """ + # Override the base class implementation so that the MultiTableMixin + # doesn't freak out. We do the processing at the TableTab level. + return {} + + def handle_table(self, table_dict): + """ + For the given dict containing a ``DataTable`` and a ``TableTab`` + instance, it loads the table data for that tab and calls the + table's :meth:`~horizon.tables.DataTable.maybe_handle` method. The + return value will be the result of ``maybe_handle``. + """ + table = table_dict['table'] + tab = table_dict['tab'] + tab.load_table_data() + table_name = table._meta.name + tab._tables[table_name]._meta.has_more_data = self.has_more_data(table) + handled = tab._tables[table_name].maybe_handle() + return handled + + def get_context_data(self, **kwargs): + """ Adds the ``tab_group`` variable to the context data. """ + context = super(TabbedTableView, self).get_context_data(**kwargs) + context['tab_group'].load_tab_data() + return context + + def get(self, request, *args, **kwargs): + self.load_tabs() + # Gather our table instances. It's important that they're the + # actual instances and not the classes! + table_instances = [t['table'] for t in self._table_dict.values()] + # Early out before any tab or table data is loaded + for table in table_instances: + preempted = table.maybe_preempt() + if preempted: + return preempted + + # If we have an action, determine if it belongs to one of our tables. + # We don't iterate through all of the tables' maybes_handle + # methods; just jump to the one that's got the matching name. + table_name, action, obj_id = tables.DataTable.check_handler(request) + if table_name in self._table_dict: + handled = self.handle_table(self._table_dict[table_name]) + if handled: + return handled + + context = self.get_context_data(**kwargs) + return self.handle_tabbed_response(context["tab_group"], context) + + def post(self, request, *args, **kwargs): + # GET and POST handling are the same + return self.get(request, *args, **kwargs) diff --git a/horizon/templates/horizon/common/_detail_table.html b/horizon/templates/horizon/common/_detail_table.html new file mode 100644 index 0000000000..99f86d4275 --- /dev/null +++ b/horizon/templates/horizon/common/_detail_table.html @@ -0,0 +1 @@ +{{ table.render }} diff --git a/horizon/tests/tabs_tests.py b/horizon/tests/tabs_tests.py index 87121e01b6..e480ebf71b 100644 --- a/horizon/tests/tabs_tests.py +++ b/horizon/tests/tabs_tests.py @@ -20,6 +20,8 @@ from django.utils.translation import ugettext_lazy as _ from horizon import tabs as horizon_tabs from horizon import test +from .table_tests import MyTable, TEST_DATA + class BaseTestTab(horizon_tabs.Tab): def get_context_data(self, request): @@ -65,6 +67,26 @@ class Group(horizon_tabs.TabGroup): self._assert_tabs_not_available = True +class TabWithTable(horizon_tabs.TableTab): + table_classes = (MyTable,) + name = _("Tab With My Table") + slug = "tab_with_table" + template_name = "horizon/common/_detail_table.html" + + def get_my_table_data(self): + return TEST_DATA + + +class TableTabGroup(horizon_tabs.TabGroup): + slug = "tab_group" + tabs = (TabWithTable,) + + +class TabWithTableView(horizon_tabs.TabbedTableView): + tab_group_class = TableTabGroup + template_name = "tab_group.html" + + class TabTests(test.TestCase): def setUp(self): super(TabTests, self).setUp() @@ -190,3 +212,56 @@ class TabTests(test.TestCase): tab_delayed = tg.get_tab("tab_delayed") output = tab_delayed.render() self.assertEqual(output.strip(), tab_delayed.name) + + def test_table_tabs(self): + tab_group = TableTabGroup(self.request) + tabs = tab_group.get_tabs() + # Only one tab, as expected. + self.assertEqual(len(tabs), 1) + tab = tabs[0] + # Make sure it's the tab we think it is. + self.assertTrue(isinstance(tab, horizon_tabs.TableTab)) + # Data should not be loaded yet. + self.assertFalse(tab._table_data_loaded) + table = tab._tables[MyTable.Meta.name] + self.assertTrue(isinstance(table, MyTable)) + # Let's make sure the data *really* isn't loaded yet. + self.assertEqual(table.data, None) + # Okay, load the data. + tab.load_table_data() + self.assertTrue(tab._table_data_loaded) + self.assertQuerysetEqual(table.data, ['', + '', + '']) + context = tab.get_context_data(self.request) + # Make sure our table is loaded into the context correctly + self.assertEqual(context['my_table_table'], table) + # Since we only had one table we should get the shortcut name too. + self.assertEqual(context['table'], table) + + def test_tabbed_table_view(self): + view = TabWithTableView.as_view() + + # Be sure we get back a rendered table containing data for a GET + req = self.factory.get("/") + res = view(req) + self.assertContains(res, "