Moved ajax updating from Action to Row.

This fixes the problem of having to "hide" the update action
in the row's actions column. Fixes bug 948397.

Additionally, added some protection for spillover between
the attrs dictionary on action instances. Fixes bug 954592.

FWIW, the code involved in the AJAX updating is largely identical,
it's just been moved from Action to Row and had few data accessors
renamed to account for the differing relation to Table and Row.
This also allowed the javascript to be significantly cleaner.

Change-Id: Ic8932a33ca6956a56c8eee09bb0a4d1f59e0ab3a
This commit is contained in:
Gabriel Hurley 2012-03-13 17:27:18 -07:00
parent f7c0dd203d
commit 753ebd673c
12 changed files with 170 additions and 181 deletions

View File

@ -62,9 +62,6 @@ Actions
.. autoclass:: LinkAction
:members:
.. autoclass:: UpdateAction
:members:
.. autoclass:: FilterAction
:members:

View File

@ -108,15 +108,13 @@ Shortcut actions
----------------
There are several common tasks for which Horizon provides pre-built shortcut
classes. These include :class:`~horizon.tables.BatchAction`,
:class:`~horizon.tables.DeleteAction`, and
:class:`~horizon.tables.UpdateAction`. Each of these abstracts away nearly
classes. These include :class:`~horizon.tables.BatchAction`, and
:class:`~horizon.tables.DeleteAction`. Each of these abstracts away nearly
all of the boilerplate associated with writing these types of actions and
provides consistent error handling, logging, and user-facing interaction.
It is worth noting that ``BatchAction`` and ``DeleteAction`` are extensions
of the standard ``Action`` class, while ``UpdateAction`` is a specialized
version of the ``LinkAction`` class.
of the standard ``Action`` class.
Preemptive actions
------------------
@ -129,7 +127,3 @@ require data, such as :meth:`~horizon.tables.DataTable.get_object_display` or
actions is that you can avoid having to do all the processing, API calls, etc.
associated with loading data into the table for actions which don't require
access to that information.
A very notable example of this is the
``UpdateAction`` type of action, which only needs information on a single
object and can do a single-object query rather than loading all the data.

View File

@ -167,8 +167,11 @@ class LogLink(tables.LinkAction):
return instance.status in ACTIVE_STATES
class UpdateRow(tables.UpdateAction):
def get_data(self, request, instance_id):
class UpdateRow(tables.Row):
ajax = True
@classmethod
def get_data(cls, 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]
@ -228,7 +231,8 @@ class InstancesTable(tables.DataTable):
name = "instances"
verbose_name = _("Instances")
status_columns = ["status", "task"]
row_class = UpdateRow
table_actions = (LaunchLink, TerminateInstance)
row_actions = (EditInstance, ConsoleLink, LogLink, SnapshotLink,
TogglePause, ToggleSuspend, RebootInstance,
TerminateInstance, UpdateRow)
TerminateInstance)

View File

@ -81,8 +81,11 @@ class CreateSnapshot(tables.LinkAction):
return volume.status == "available"
class UpdateRow(tables.UpdateAction):
def get_data(self, request, volume_id):
class UpdateRow(tables.Row):
ajax = True
@classmethod
def get_data(cls, request, volume_id):
volume = api.volume_get(request, volume_id)
return volume
@ -137,9 +140,9 @@ class VolumesTable(VolumesTableBase):
name = "volumes"
verbose_name = _("Volumes")
status_columns = ["status"]
row_class = UpdateRow
table_actions = (CreateVolume, DeleteVolume,)
row_actions = (EditAttachments, CreateSnapshot,
DeleteVolume, UpdateRow)
row_actions = (EditAttachments, CreateSnapshot, DeleteVolume)
class DetachVolume(tables.BatchAction):

View File

@ -17,18 +17,14 @@
import logging
from django import template
from django.template.defaultfilters import title
from django.utils.datastructures import SortedDict
from django.utils.translation import ugettext as _
from horizon import api
from horizon import tables
from horizon.dashboards.nova.instances_and_volumes.instances.tables import \
(LaunchLink, TerminateInstance, EditInstance, ConsoleLink, LogLink,
SnapshotLink, TogglePause, ToggleSuspend, RebootInstance, get_size,
TerminateInstance, UpdateRow, get_ips, get_power_state)
from horizon.templatetags import sizeformat
LOG = logging.getLogger(__name__)
@ -71,6 +67,7 @@ class SyspanelInstancesTable(tables.DataTable):
verbose_name = _("Instances")
status_columns = ["status", "task"]
table_actions = (TerminateInstance,)
row_class = UpdateRow
row_actions = (EditInstance, ConsoleLink, LogLink, SnapshotLink,
TogglePause, ToggleSuspend, RebootInstance,
TerminateInstance, UpdateRow)
TerminateInstance)

View File

@ -43,23 +43,41 @@ var Horizon = function() {
/* Namespace for core functionality related to DataTables. */
horizon.datatables = {
update: function () {
var rows_to_update = $('tr.status_unknown');
if (rows_to_update.length) {
var $updaters = rows_to_update.find('.ajax-update');
var interval = $updaters.attr('data-update-interval');
var $table = rows_to_update.closest('table');
var decay_constant = $table.attr('decay_constant');
var $rows_to_update = $('tr.status_unknown.ajax-update');
if ($rows_to_update.length) {
var interval = $rows_to_update.attr('data-update-interval'),
$table = $rows_to_update.closest('table'),
decay_constant = $table.attr('decay_constant');
// Do not update this row if the action column is expanded
if (rows_to_update.find('.actions_column .btn-group.open').length) {
if ($rows_to_update.find('.actions_column .btn-group.open').length) {
// Wait and try to update again in next interval instead
setTimeout(horizon.datatables.update, interval);
// Remove interval decay, since this will not hit server
$table.removeAttr('decay_constant');
return;
}
// Trigger the update handler.
$updaters.click();
// Trigger the update handlers.
$rows_to_update.each(function(index, row) {
var $row = $(this);
$.ajax($row.attr('data-update-url'), {
complete: function (jqXHR, status) {
var $new_row = $(jqXHR.responseText);
$new_row.find("td.status_unknown").prepend('<i class="icon-updating ajax-updating"></i>');
// Only replace row if the html content has changed
if($new_row.html() != $row.html()) {
if($row.find(':checkbox').is(':checked')) {
// Preserve the checkbox if it's already clicked
$new_row.find(':checkbox').prop('checked', true);
}
$row.replaceWith($new_row);
$table.removeAttr('decay_constant');
}
// Revalidate the button check for updated table
horizon.datatables.validate_button();
}
});
});
// Set interval decay to this table, and increase if it already exist
if(decay_constant === undefined) {
@ -69,9 +87,9 @@ var Horizon = function() {
}
$table.attr('decay_constant', decay_constant);
// Poll until there are no rows in an "unknown" state on the page.
next_poll = interval*decay_constant;
next_poll = interval * decay_constant;
// Limit the interval to 30 secs
if(next_poll > 30*1000) next_poll = 30*1000;
if(next_poll > 30 * 1000) next_poll = 30 * 1000;
setTimeout(horizon.datatables.update, next_poll);
}
},
@ -79,11 +97,11 @@ var Horizon = function() {
// Disable form button if checkbox are not checked
$("form").each(function (i) {
var checkboxes = $(this).find(":checkbox");
if(checkboxes.length == 0) {
if(!checkboxes.length) {
// Do nothing if no checkboxes in this form
return;
}
if(checkboxes.filter(":checked").length == 0) {
if(!checkboxes.filter(":checked").length) {
$(this).find(".table_actions button.btn-danger").addClass("disabled");
}
});
@ -113,7 +131,7 @@ var Horizon = function() {
action_string = $action.text();
title = "Confirm " + action_string;
body = "Please confirm your selection. This action cannot be undone.";
var use_backdrop = $('.modal').length == 0; // check if already has a modal
var use_backdrop = !$('.modal').length; // check if already has a modal
modal = horizon.modals.create(title, body, action_string);
modal.modal({backdrop: use_backdrop});
modal.find('.btn-primary').click(function (evt) {

View File

@ -26,29 +26,5 @@ horizon.addInitFunction(function() {
}
});
$('table').on('click', 'tr .ajax-update', function (evt) {
var $this = $(this);
var $table = $this.closest('table');
$.ajax($this.attr('href'), {
complete: function (jqXHR, status) {
var $new_row = $(jqXHR.responseText);
var $old_row = $this.closest('tr');
$new_row.find("td.status_unknown").prepend('<i class="icon-updating ajax-updating"></i>');
// Only replace row if the html content has changed
if($new_row.html() != $old_row.html()) {
if($old_row.find(':checkbox').is(':checked')) {
// Preserve the checkbox if it's already clicked
$new_row.find(':checkbox').prop('checked', true);
}
$old_row.replaceWith($new_row);
$table.removeAttr('decay_constant');
}
// Revalidate the button check for updated table
horizon.datatables.validate_button();
}
});
return false;
});
horizon.datatables.update();
});

View File

@ -16,6 +16,6 @@
# Convenience imports for public API components.
from .actions import (Action, BatchAction, DeleteAction,
LinkAction, FilterAction, UpdateAction)
LinkAction, FilterAction)
from .base import DataTable, Column, Row
from .views import DataTableView, MultiTableView

View File

@ -16,16 +16,12 @@
import logging
import new
from urlparse import urlparse
from urlparse import parse_qs
from django import http
from django import shortcuts
from django.conf import settings
from django.contrib import messages
from django.core import urlresolvers
from django.utils.functional import Promise
from django.utils.http import urlencode
from django.utils.translation import string_concat, ugettext as _
from horizon import exceptions
@ -34,7 +30,7 @@ from horizon.utils import html
LOG = logging.getLogger(__name__)
# For Bootstrap integration, can be overridden in settings.
# For Bootstrap integration; can be overridden in settings.
ACTION_CSS_CLASSES = ("btn", "btn-small")
@ -256,77 +252,6 @@ class LinkAction(BaseAction):
return self.url
class UpdateAction(LinkAction):
""" A base class for handling updating rows on tables with new data.
Subclasses need to define a ``get_data`` method which returns a data
object appropriate for consumption by the table (effectively the "get"
lookup versus the table's "list" lookup).
By default, this action is meant to be a row-level action, and is hidden
from the row's action list. It is instead triggered via automatic AJAX
updates based on the row status.
The automatic update interval is determined first by setting the key
``ajax_poll_interval`` in the ``settings.HORIZON_CONFIG`` dictionary.
If that key is not present, it falls back to the value of the
``update_interval`` attribute on this class.
Default: ``2500`` (measured in milliseconds).
"""
name = "update"
verbose_name = _("Update")
method = "GET"
classes = ('ajax-update', 'hide')
preempt = True
update_interval = 2500
def __init__(self, *args, **kwargs):
super(UpdateAction, self).__init__(*args, **kwargs)
interval = settings.HORIZON_CONFIG.get('ajax_poll_interval',
self.update_interval)
self.attrs['data-update-interval'] = interval
def get_link_url(self, datum=None):
table_url = self.table.get_absolute_url()
query = parse_qs(urlparse(table_url).query)
# Strip the query off, since we're adding a different action
# here, and the existing query may have an action. This is not
# ideal because it prevents other uses of the querystring, but
# it does prevent runaway compound querystring construction.
if 'action' in query:
table_url = table_url.partition('?')[0]
params = urlencode({'table': self.table.name,
'action': self.name,
'obj_id': self.table.get_object_id(datum)})
return "%s?%s" % (table_url, params)
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.
"""
raise NotImplementedError("You must define a get_data method on %s"
% self.__class__.__name__)
def single(self, data_table, request, obj_id):
try:
datum = self.get_data(request, obj_id)
error = False
except:
datum = None
error = exceptions.handle(request, ignore=True)
if request.is_ajax():
if not error:
row = data_table._meta.row_class(data_table, datum)
return http.HttpResponse(row.render())
else:
return http.HttpResponse(status=error.status_code)
# NOTE(gabriel): returning None from the action continues
# with the view as normal. This will generally be the equivalent
# of refreshing the page.
return None
class FilterAction(BaseAction):
""" A base class representing a filter action for a table.

View File

@ -21,6 +21,7 @@ from operator import attrgetter
import sys
from django import forms
from django.http import HttpResponse
from django import template
from django.conf import settings
from django.contrib import messages
@ -29,11 +30,13 @@ from django.template.loader import render_to_string
from django.utils import http
from django.utils.datastructures import SortedDict
from django.utils.html import escape
from django.utils.http import urlencode
from django.utils.translation import ugettext as _
from django.utils.safestring import mark_safe
from django.utils import termcolors
from horizon import exceptions
from horizon.utils import html
from .actions import FilterAction, LinkAction
@ -239,11 +242,22 @@ class Column(object):
return self.link
class Row(object):
class Row(html.HTMLElement):
""" Represents a row in the table.
When iterated, the ``Row`` instance will yield each of its cells.
Rows are capable of AJAX updating, with a little added work:
The ``ajax`` property needs to be set to ``True``, and
subclasses need to define a ``get_data`` method which returns a data
object appropriate for consumption by the table (effectively the "get"
lookup versus the table's "list" lookup).
The automatic update interval is configurable by setting the key
``ajax_poll_interval`` in the ``settings.HORIZON_CONFIG`` dictionary.
Default: ``2500`` (measured in milliseconds).
.. attribute:: table
The table which this row belongs to.
@ -270,16 +284,36 @@ class Row(object):
.. attribute:: status_class
Returns a css class for the status of the row based on ``status``.
.. attribute:: ajax
Boolean value to determine whether ajax updating for this row is
enabled.
.. attribute:: ajax_action_name
String that is used for the query parameter key to request AJAX
updates. Generally you won't need to change this value.
Default: ``"row_update"``.
"""
ajax = False
ajax_action_name = "row_update"
def __init__(self, table, datum):
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")
# Compile all the cells on instantiation
# Compile all the cells on instantiation.
cells = []
for column in table.columns.values():
if column.auto == "multi_select":
@ -297,6 +331,16 @@ class Row(object):
cells.append((column.name or column.auto, cell))
self.cells = SortedDict(cells)
# Add the row's status class and id to the attributes to be rendered.
self.classes.append(self.status_class)
self.attrs['id'] = self.id
def __repr__(self):
return '<%s: %s>' % (self.__class__.__name__, self.id)
def __iter__(self):
return iter(self.cells.values())
@property
def status(self):
column_names = self.table._meta.status_columns
@ -321,11 +365,21 @@ class Row(object):
""" Returns the bound cells for this row in order. """
return self.cells.values()
def __repr__(self):
return '<%s: %s>' % (self.__class__.__name__, self.id)
def get_ajax_update_url(self):
table_url = self.table.get_absolute_url()
params = urlencode({"table": self.table.name,
"action": self.ajax_action_name,
"obj_id": self.table.get_object_id(self.datum)})
return "%s?%s" % (table_url, params)
def __iter__(self):
return iter(self.cells.values())
@classmethod
def get_data(cls, 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__)
class Cell(object):
@ -686,10 +740,10 @@ class DataTable(object):
after a successful action on the table.
For convenience it defaults to the value of
``request.get_full_path()``, e.g. the path at which the table
was requested.
``request.get_full_path()`` with any query string stripped off,
e.g. the path at which the table was requested.
"""
return self._meta.request.get_full_path()
return self._meta.request.get_full_path().partition('?')[0]
def get_empty_message(self):
""" Returns the message to be displayed when there is no data. """
@ -727,6 +781,7 @@ class DataTable(object):
for action in self._meta.row_actions:
# Copy to allow modifying properties per row
bound_action = copy.copy(self.base_actions[action.name])
bound_action.attrs = copy.copy(bound_action.attrs)
# Remove disallowed actions.
if not self._filter_action(bound_action,
self._meta.request,
@ -818,6 +873,7 @@ class DataTable(object):
def _check_handler(self):
""" 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"])
elif "table" in request.GET and "action" in request.GET:
@ -831,17 +887,35 @@ class DataTable(object):
def maybe_preempt(self):
"""
Determine whether the request should be handled by a preemptive action
on this table before loading any data.
on this table or by an AJAX row update before loading any data.
"""
table_name, action_name, obj_id = self._check_handler()
preemptive_actions = [action for action in self.base_actions.values()
if action.preempt]
if table_name == self.name and action_name:
for action in preemptive_actions:
if action.name == action_name:
handled = self.take_action(action_name, obj_id)
if handled:
return handled
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:
try:
datum = row_class.get_data(self._meta.request, obj_id)
error = False
except:
datum = None
error = exceptions.handle(self._meta.request, ignore=True)
if self._meta.request.is_ajax():
if not error:
row = row_class(self, datum)
return HttpResponse(row.render())
else:
return HttpResponse(status=error.status_code)
preemptive_actions = [action for action in
self.base_actions.values() if action.preempt]
if action_name:
for action in preemptive_actions:
if action.name == action_name:
handled = self.take_action(action_name, obj_id)
if handled:
return handled
return None
def maybe_handle(self):

View File

@ -1,3 +1,3 @@
<tr id="{{ row.id }}" class="{{ row.status_class }}">
<tr {{ row.attr_string|safe }}>
{% for cell in row %}<td class="{{ cell.get_classes }}">{{ cell.value }}</td>{% endfor %}
</tr>

View File

@ -76,8 +76,11 @@ class MyAction(tables.Action):
return shortcuts.redirect('http://example.com/%s' % len(object_ids))
class MyUpdateAction(tables.UpdateAction):
def get_data(self, request, obj_id):
class MyRow(tables.Row):
ajax = True
@classmethod
def get_data(cls, request, obj_id):
return TEST_DATA_2[0]
@ -149,9 +152,9 @@ class MyTable(tables.DataTable):
verbose_name = "My Table"
status_columns = ["status"]
columns = ('id', 'name', 'value', 'optional', 'status')
row_class = MyRow
table_actions = (MyFilterAction, MyAction, MyBatchAction)
row_actions = (MyAction, MyLinkAction, MyUpdateAction,
MyBatchAction, MyToggleAction)
row_actions = (MyAction, MyLinkAction, MyBatchAction, MyToggleAction)
class DataTableTests(test.TestCase):
@ -183,8 +186,7 @@ class DataTableTests(test.TestCase):
'<MyAction: delete>',
'<MyFilterAction: filter>',
'<MyLinkAction: login>',
'<MyToggleAction: toggle>',
'<MyUpdateAction: update>'])
'<MyToggleAction: toggle>'])
self.assertQuerysetEqual(self.table.get_table_actions(),
['<MyFilterAction: filter>',
'<MyAction: delete>',
@ -192,7 +194,6 @@ class DataTableTests(test.TestCase):
self.assertQuerysetEqual(self.table.get_row_actions(TEST_DATA[0]),
['<MyAction: delete>',
'<MyLinkAction: login>',
'<MyUpdateAction: update>',
'<MyBatchAction: batch>',
'<MyToggleAction: toggle>'])
# Auto-generated columns
@ -281,9 +282,9 @@ class DataTableTests(test.TestCase):
'<Column: actions>'])
# Verify we retrieve the right rows from our data
rows = self.table.get_rows()
self.assertQuerysetEqual(rows, ['<Row: my_table__row__1>',
'<Row: my_table__row__2>',
'<Row: my_table__row__3>'])
self.assertQuerysetEqual(rows, ['<MyRow: my_table__row__1>',
'<MyRow: my_table__row__2>',
'<MyRow: my_table__row__3>'])
# Verify each row contains the right cells
self.assertQuerysetEqual(rows[0].get_cells(),
['<Cell: multi_select, my_table__row__1>',
@ -372,11 +373,8 @@ class DataTableTests(test.TestCase):
# Row actions
row_actions = self.table.render_row_actions(TEST_DATA[0])
resp = http.HttpResponse(row_actions)
self.assertContains(resp, "<li", 4)
self.assertContains(resp, "<li", 3)
self.assertContains(resp, "my_table__delete__1", 1)
self.assertContains(resp,
"action=update&amp;table=my_table&amp;obj_id=1", 1)
self.assertContains(resp, "data-update-interval", 1)
self.assertContains(resp, "my_table__toggle__1", 1)
self.assertContains(resp, "/auth/login/", 1)
self.assertContains(resp, "ajax-modal", 1)
@ -384,9 +382,12 @@ class DataTableTests(test.TestCase):
resp = http.HttpResponse(self.table.render())
self.assertContains(resp, '<table id="my_table"', 1)
self.assertContains(resp, '<th ', 7)
self.assertContains(resp, '<tr id="my_table__row__1"', 1)
self.assertContains(resp, '<tr id="my_table__row__2"', 1)
self.assertContains(resp, '<tr id="my_table__row__3"', 1)
self.assertContains(resp, 'id="my_table__row__1"', 1)
self.assertContains(resp, 'id="my_table__row__2"', 1)
self.assertContains(resp, 'id="my_table__row__3"', 1)
update_string = "action=row_update&amp;table=my_table&amp;obj_id="
self.assertContains(resp, update_string, 3)
self.assertContains(resp, "data-update-interval", 3)
# Verify our XSS protection
self.assertContains(resp, '<a href="http://example.com/">'
'&lt;strong&gt;evil&lt;/strong&gt;</a>', 1)
@ -410,15 +411,15 @@ class DataTableTests(test.TestCase):
# Batch action (without toggle) conjugation behavior
req = self.factory.get('/my_url/')
self.table = MyTable(req, TEST_DATA_3)
toggle_action = self.table.get_row_actions(TEST_DATA_3[0])[3]
toggle_action = self.table.get_row_actions(TEST_DATA_3[0])[2]
self.assertEqual(unicode(toggle_action.verbose_name), "Batch Item")
# Single object toggle action
# GET page - 'up' to 'down'
req = self.factory.get('/my_url/')
self.table = MyTable(req, TEST_DATA_3)
self.assertEqual(len(self.table.get_row_actions(TEST_DATA_3[0])), 5)
toggle_action = self.table.get_row_actions(TEST_DATA_3[0])[4]
self.assertEqual(len(self.table.get_row_actions(TEST_DATA_3[0])), 4)
toggle_action = self.table.get_row_actions(TEST_DATA_3[0])[3]
self.assertEqual(unicode(toggle_action.verbose_name), "Down Item")
# Toggle from status 'up' to 'down'
@ -438,8 +439,8 @@ class DataTableTests(test.TestCase):
# GET page - 'down' to 'up'
req = self.factory.get('/my_url/')
self.table = MyTable(req, TEST_DATA_2)
self.assertEqual(len(self.table.get_row_actions(TEST_DATA_2[0])), 4)
toggle_action = self.table.get_row_actions(TEST_DATA_2[0])[3]
self.assertEqual(len(self.table.get_row_actions(TEST_DATA_2[0])), 3)
toggle_action = self.table.get_row_actions(TEST_DATA_2[0])[2]
self.assertEqual(unicode(toggle_action.verbose_name), "Up Item")
# POST page
@ -502,7 +503,7 @@ class DataTableTests(test.TestCase):
['<FakeObject: object_2>'])
# Updating and preemptive actions
params = {"table": "my_table", "action": "update", "obj_id": "1"}
params = {"table": "my_table", "action": "row_update", "obj_id": "1"}
req = self.factory.get('/my_url/',
params,
HTTP_X_REQUESTED_WITH='XMLHttpRequest')