Batch actions (including Delete) for DataTables.

This patch implements a new BatchAction for data tables that simplifies error handling, messaging, and standardizes behavior.

Change-Id: I19195095add9b824a48e4f2078d898344f92765b
This commit is contained in:
Paul McMillan
2012-01-12 13:29:39 -08:00
parent a61734e730
commit 1b858a55ed
10 changed files with 246 additions and 161 deletions

View File

@@ -50,28 +50,15 @@ class AllocateIP(tables.Action):
return shortcuts.redirect('horizon:nova:access_and_security:index')
class ReleaseIP(tables.Action):
class ReleaseIPs(tables.BatchAction):
name = "release"
verbose_name = _("Release IP")
classes = ('danger',)
action_present = _("Release")
action_past = _("Released")
data_type_singular = _("Floating IP")
data_type_plural = _("Floating IPs")
def handle(self, table, request, object_ids):
released = []
for obj_id in object_ids:
LOG.info('Releasing Floating IP "%s".' % obj_id)
try:
api.tenant_floating_ip_release(request, obj_id)
# Floating IP ids are returned from the API as integers
released.append(table.get_object_by_id(int(obj_id)))
except novaclient_exceptions.ClientException, e:
LOG.exception("ClientException in ReleaseFloatingIp")
messages.error(request, _('Unable to release Floating IP '
'from tenant.'))
if released:
messages.info(request,
_('Successfully released floating IPs: %s.')
% ", ".join([ip.ip for ip in released]))
return shortcuts.redirect('horizon:nova:access_and_security:index')
def action(self, request, obj_id):
api.tenant_floating_ip_release(request, obj_id)
class AssociateIP(tables.LinkAction):
@@ -116,8 +103,14 @@ class FloatingIPsTable(tables.DataTable):
verbose_name=_("Instance"),
empty_value="-")
def sanitize_id(self, obj_id):
return int(obj_id)
def get_object_display(self, datum):
return datum.ip
class Meta:
name = "floating_ips"
verbose_name = _("Floating IPs")
table_actions = (AllocateIP, ReleaseIP)
row_actions = (AssociateIP, DisassociateIP, ReleaseIP)
table_actions = (AllocateIP, ReleaseIPs)
row_actions = (AssociateIP, DisassociateIP, ReleaseIPs)

View File

@@ -27,30 +27,12 @@ from horizon import tables
LOG = logging.getLogger(__name__)
class DeleteKeyPairs(tables.Action):
name = "delete"
verbose_name = _("Delete")
verbose_name_plural = _("Delete Keypairs")
classes = ("danger",)
class DeleteKeyPairs(tables.DeleteAction):
data_type_singular = _("Keypair")
data_type_plural = _("Keypairs")
def handle(self, data_table, request, object_ids):
failures = 0
deleted = []
for obj_id in object_ids:
try:
api.nova.keypair_delete(request, obj_id)
deleted.append(obj_id)
except Exception, e:
failures += 1
messages.error(request, _("Error deleting keypair: %s") % e)
LOG.exception("Error deleting keypair.")
if failures:
messages.info(request, _("Deleted the following keypairs: %s")
% ", ".join(deleted))
else:
messages.success(request, _("Successfully deleted keypairs: %s")
% ", ".join(deleted))
return shortcuts.redirect('horizon:nova:access_and_security:index')
def delete(self, request, obj_id):
api.nova.keypair_delete(request, obj_id)
class ImportKeyPair(tables.LinkAction):

View File

@@ -28,38 +28,17 @@ from horizon import tables
LOG = logging.getLogger(__name__)
class DeleteGroup(tables.Action):
name = "delete"
verbose_name = _("Delete")
verbose_name_plural = _("Delete Security Groups")
classes = ('danger',)
class DeleteGroup(tables.DeleteAction):
data_type_singular = _("Security Group")
data_type_plural = _("Security Groups")
def allowed(self, request, security_group=None):
if not security_group:
return True
return security_group.name != 'default'
def handle(self, table, request, object_ids):
tenant_id = request.user.tenant_id
deleted = []
for obj_id in object_ids:
obj = table.get_object_by_id(int(obj_id))
if obj.name == "default":
messages.info(request, _("The default group can't be deleted"))
continue
try:
security_group = api.security_group_delete(request, obj_id)
deleted.append(obj)
LOG.info('Deleted security_group: "%s"' % obj.name)
except novaclient_exceptions.ClientException, e:
LOG.exception("Error deleting security group")
messages.error(request, _('Unable to delete group: %s')
% obj.name)
if deleted:
messages.success(request,
_('Successfully deleted security groups: %s')
% ", ".join([group.name for group in deleted]))
return shortcuts.redirect('horizon:nova:access_and_security:index')
def delete(self, request, obj_id):
api.security_group_delete(request, obj_id)
class CreateGroup(tables.LinkAction):
@@ -79,6 +58,9 @@ class SecurityGroupsTable(tables.DataTable):
name = tables.Column("name")
description = tables.Column("description")
def sanitize_id(self, obj_id):
return int(obj_id)
class Meta:
name = "security_groups"
verbose_name = _("Security Groups")
@@ -86,26 +68,12 @@ class SecurityGroupsTable(tables.DataTable):
row_actions = (EditRules, DeleteGroup)
class DeleteRule(tables.Action):
name = "delete"
verbose_name = _("Delete")
classes = ('danger',)
class DeleteRule(tables.DeleteAction):
data_type_singular = _("Security Group Rule")
data_type_plural = _("Security Group Rules")
def single(self, table, request, obj_id):
tenant_id = request.user.tenant_id
obj = table.get_object_by_id(int(obj_id))
try:
LOG.info('Delete security_group_rule: "%s"' % obj_id)
security_group = api.security_group_rule_delete(request, obj_id)
messages.info(request, _('Successfully deleted rule: %s')
% obj_id)
except novaclient_exceptions.ClientException, e:
LOG.exception("ClientException in DeleteRule")
messages.error(request, _('Error authorizing security group: %s')
% e.message)
return shortcuts.redirect('horizon:nova:access_and_security:'
'security_groups:edit_rules',
(obj.parent_group_id))
def delete(self, request, obj_id):
api.security_group_rule_delete(request, obj_id)
def get_cidr(rule):
@@ -118,6 +86,14 @@ class RulesTable(tables.DataTable):
to_port = tables.Column("to_port", verbose_name=_("To Port"))
cidr = tables.Column(get_cidr, verbose_name=_("CIDR"))
def sanitize_id(self, obj_id):
return int(obj_id)
def get_object_display(self, datum):
#FIXME (PaulM) Do something prettier here
return ', '.join([':'.join((k, str(v))) for
k, v in datum._apidict.iteritems()])
class Meta:
name = "rules"
verbose_name = _("Security Group Rules")

View File

@@ -19,6 +19,7 @@
# under the License.
from django import http
from django.conf import settings
from django.core.urlresolvers import reverse
from glance.common import exception as glance_exception
from openstackx.api import exceptions as api_exceptions
@@ -29,7 +30,6 @@ from horizon import api
from horizon import test
from .tables import SecurityGroupsTable, RulesTable
SECGROUP_ID = '2'
INDEX_URL = reverse('horizon:nova:access_and_security:index')
SG_CREATE_URL = \
@@ -39,6 +39,10 @@ SG_EDIT_RULE_URL = \
args=[SECGROUP_ID])
def strip_absolute_base(uri):
return uri.split(settings.TESTSERVER, 1)[-1]
class SecurityGroupsViewTests(test.BaseViewTests):
def setUp(self):
super(SecurityGroupsViewTests, self).setUp()
@@ -219,7 +223,8 @@ class SecurityGroupsViewTests(test.BaseViewTests):
table = RulesTable(req, self.rules)
handled = table.maybe_handle()
self.assertEqual(handled['location'], SG_EDIT_RULE_URL)
self.assertEqual(strip_absolute_base(handled['location']),
SG_EDIT_RULE_URL)
def test_edit_rules_delete_rule_exception(self):
RULE_ID = '1'
@@ -238,7 +243,8 @@ class SecurityGroupsViewTests(test.BaseViewTests):
table = RulesTable(req, self.rules)
handled = table.maybe_handle()
self.assertEqual(handled['location'], SG_EDIT_RULE_URL)
self.assertEqual(strip_absolute_base(handled['location']),
SG_EDIT_RULE_URL)
def test_delete_group(self):
self.mox.StubOutWithMock(api, 'security_group_delete')
@@ -251,7 +257,8 @@ class SecurityGroupsViewTests(test.BaseViewTests):
table = SecurityGroupsTable(req, self.security_groups)
handled = table.maybe_handle()
self.assertEqual(handled['location'], INDEX_URL)
self.assertEqual(strip_absolute_base(handled['location']),
INDEX_URL)
def test_delete_group_exception(self):
self.mox.StubOutWithMock(api, 'security_group_delete')
@@ -267,4 +274,5 @@ class SecurityGroupsViewTests(test.BaseViewTests):
table = SecurityGroupsTable(req, self.security_groups)
handled = table.maybe_handle()
self.assertEqual(handled['location'], INDEX_URL)
self.assertEqual(strip_absolute_base(handled['location']),
INDEX_URL)

View File

@@ -38,31 +38,12 @@ class CreateLink(tables.LinkAction):
attrs = {"class": "ajax-modal btn small"}
class DeleteTenantsAction(tables.Action):
name = "delete"
verbose_name = _("Delete")
verbose_name_plural = _("Delete Tenants")
classes = ("danger",)
class DeleteTenantsAction(tables.DeleteAction):
data_type_singular = _("Tenant")
data_type_plural = _("Tenants")
def handle(self, data_table, request, object_ids):
failures = 0
deleted = []
for obj_id in object_ids:
LOG.info('Deleting tenant with id "%s"' % obj_id)
try:
api.keystone.tenant_delete(request, obj_id)
deleted.append(obj_id)
except Exception, e:
failures += 1
messages.error(request, _("Error deleting tenant: %s") % e)
LOG.exception("Error deleting tenant.")
if failures:
messages.info(request, _("Deleted the following tenant: %s")
% ", ".join(deleted))
else:
messages.success(request, _("Successfully deleted tenant: %s")
% ", ".join(deleted))
return shortcuts.redirect('horizon:syspanel:tenants:index')
def delete(self, request, obj_id):
api.keystone.tenant_delete(request, obj_id)
class TenantFilterAction(tables.FilterAction):

View File

@@ -91,36 +91,17 @@ class DisableUsersAction(tables.Action):
return shortcuts.redirect('horizon:syspanel:users:index')
class DeleteUsersAction(tables.Action):
name = "delete"
verbose_name = _("Delete")
verbose_name_plural = _("Delete Users")
classes = ("danger",)
class DeleteUsersAction(tables.DeleteAction):
data_type_singular = _("User")
data_type_plural = _("Users")
def handle(self, data_table, request, object_ids):
failures = 0
deleted = []
for obj_id in object_ids:
if obj_id == request.user.id:
messages.info(request, _('You cannot delete the user you are '
'currently logged in as.'))
continue
LOG.info('Deleting user with id "%s"' % obj_id)
try:
api.keystone.user_delete(request, obj_id)
deleted.append(obj_id)
except Exception, e:
failures += 1
messages.error(request, _("Error deleting user: %s") % e)
LOG.exception("Error deleting user.")
if failures:
messages.info(request, _("Deleted the following users: %s")
% ", ".join(deleted))
else:
if deleted:
messages.success(request, _("Successfully deleted users: %s")
% ", ".join(deleted))
return shortcuts.redirect('horizon:syspanel:users:index')
def allowed(self, request, datum):
if datum and datum.id == request.user.id:
return False
return True
def delete(self, request, obj_id):
api.keystone.user_delete(request, obj_id)
class UserFilterAction(tables.FilterAction):

View File

@@ -109,11 +109,10 @@ class UsersViewTests(test.BaseAdminViewTests):
formData = {'action': 'users__disable__%s' % self.request.user.id}
res = self.client.post(USERS_INDEX_URL, formData, follow=True)
self.assertEqual(list(res.context['messages'])[0].message,
'You cannot disable the user you are currently '
'logged in as.')
u'You cannot disable the user you are currently '
u'logged in as.')
formData = {'action': 'users__delete__%s' % self.request.user.id}
res = self.client.post(USERS_INDEX_URL, formData, follow=True)
self.assertEqual(list(res.context['messages'])[0].message,
'You cannot delete the user you are currently '
'logged in as.')
u'You do not have permission to delete user: test')

View File

@@ -15,6 +15,7 @@
# under the License.
# Convenience imports for public API components.
from .actions import Action, LinkAction, FilterAction
from .actions import (Action, BatchAction, DeleteAction,
LinkAction, FilterAction)
from .base import DataTable, Column
from .views import DataTableView, MultiTableView

View File

@@ -17,8 +17,11 @@
import logging
import new
from django import shortcuts
from django.forms.util import flatatt
from django.contrib import messages
from django.core import urlresolvers
from django.utils.translation import string_concat
LOG = logging.getLogger(__name__)
@@ -100,7 +103,6 @@ class Action(BaseAction):
Handler for multi-object actions.
.. method:: handle(self, data_table, request, object_ids)
If a single function can work for both single-object and
@@ -151,8 +153,10 @@ class Action(BaseAction):
self.handles_multiple = True
if not has_handler and (not has_single or has_multiple):
raise ValueError('You must define either a "handle" method '
' or a "single" or "multiple" method.')
raise NotImplementedError('You must define either a "handle" '
'method or a "single" or "multiple"'
' method.')
if not has_single:
def single(self, data_table, request, object_id):
return self.handle(data_table, request, [object_id])
@@ -198,11 +202,11 @@ class LinkAction(BaseAction):
verbose_name))
self.url = getattr(self, "url", url)
if not self.verbose_name:
raise ValueError('A LinkAction object must have a '
'verbose_name attribute.')
raise NotImplementedError('A LinkAction object must have a '
'verbose_name attribute.')
if not self.url:
raise ValueError('A LinkAction object must have a '
'url attribute.')
raise NotImplementedError('A LinkAction object must have a '
'url attribute.')
if attrs:
self.attrs.update(attrs)
@@ -272,3 +276,145 @@ class FilterAction(BaseAction):
"""
raise NotImplementedError("The filter method has not been implemented "
"by %s." % self.__class__)
class BatchAction(Action):
""" A table action which takes batch action on one or more
objects. This action should not require user input on a
per-object basis.
.. attribute:: name
An internal name for this action.
.. attribute:: action_present
The display form of the name. Should be a transitive verb,
capitalized and translated. ("Delete", "Rotate", etc.)
.. attribute:: action_past
The past tense of action_present. ("Deleted", "Rotated", etc.)
.. attribute:: data_type_singular
A display name for the type of data that receives the
action. ("Keypair", "Floating IP", etc.)
.. attribute:: data_type_plural
Optional plural word for the type of data being acted
on. Defaults to appending 's'. Relying on the default is bad
for translations and should not be done.
.. attribute:: success_url
Optional location to redirect after completion of the delete
action. Defaults to the current page.
.. method:: get_success_url(self, request=None)
Optional method that returns the success url.
.. method:: action(self, request, datum_id)
Required method that accepts the specified object information
and performs the action. Return values are discarded, errors
raised are caught and logged.
.. method:: allowed(self, request, datum)
Optional method that returns a boolean indicating whether the
action is allowed for the given input.
"""
completion_url = None
def _conjugate(self, items=None, past=False):
"""Builds combinations like 'Delete Object' and 'Deleted
Objects' based on the number of items and `past` flag.
"""
if past:
action = self.action_past
else:
action = self.action_present
if items is None or len(items) == 1:
data_type = self.data_type_singular
else:
data_type = self.data_type_plural
return string_concat(action, ' ', data_type)
def __init__(self):
self.data_type_plural = getattr(self, 'data_type_plural',
self.data_type_singular + 's')
self.verbose_name = getattr(self, 'verbose_name',
self._conjugate())
self.verbose_name_plural = getattr(self, 'verbose_name_plural',
self._conjugate('plural'))
super(BatchAction, self).__init__()
def action(self, request, datum_id):
""" Override to take action on the specified datum. Return
values are ignored, errors raised are caught and logged.
"""
raise NotImplementedError('action() must be defined for '
'BatchAction: %s' % self.data_type_singular)
def get_completion_url(self, request=None):
if self.completion_url:
return self.completion_url
return request.build_absolute_uri()
def handle(self, table, request, obj_ids):
tenant_id = request.user.tenant_id
action_success = []
action_failure = []
action_not_allowed = []
for datum_id in obj_ids:
datum = table.get_object_by_id(datum_id)
datum_display = table.get_object_display(datum)
if not table._filter_action(self, request, datum):
action_not_allowed.append(datum_display)
LOG.info('Permission denied to %s: "%s"' %
(self._conjugate(past=True).lower(), datum_display))
continue
try:
self.action(request, datum_id)
action_success.append(datum_display)
LOG.info('%s: "%s"' %
(self._conjugate(past=True), datum_display))
except Exception, e:
action_failure.append(datum_display)
LOG.exception("Unable to %s: %s" %
(self._conjugate().lower(), e))
#Begin with success message class, downgrade to info if problems
success_message_level = messages.success
if action_not_allowed:
messages.error(request, _('You do not have permission to %s: %s') %
(self._conjugate(action_not_allowed).lower(),
", ".join(action_not_allowed)))
success_message_level = messages.info
if action_failure:
messages.error(request, _('Unable to %s: %s') % (
self._conjugate(action_failure).lower(),
", ".join(action_failure)))
success_message_level = messages.info
if action_success:
success_message_level(request, _('%s: %s') % (
self._conjugate(action_success, True),
", ".join(action_success)))
return shortcuts.redirect(self.get_completion_url(request))
class DeleteAction(BatchAction):
name = "delete"
action_present = _("Delete")
action_past = _("Deleted")
classes = ('danger',)
def action(self, request, obj_id):
return self.delete(request, obj_id)
def delete(self, request, obj_id):
raise NotImplementedError("DeleteAction must define a delete method.")

View File

@@ -637,7 +637,7 @@ class DataTable(object):
"""
Returns the data object from the table's dataset which matches
the ``lookup`` parameter specified. An error will be raised if
a the match is not a single data object.
the match is not a single data object.
Uses :meth:`~horizon.tables.DataTable.get_object_id` internally.
"""
@@ -728,6 +728,10 @@ class DataTable(object):
obj_ids = obj_ids or self._meta.request.POST.getlist('object_ids')
action = self.base_actions.get(action_name, None)
if action and (not action.requires_input or obj_id or obj_ids):
if obj_id:
obj_id = self.sanitize_id(obj_id)
if obj_ids:
obj_ids = [self.sanitize_id(i) for i in obj_ids]
# Single handling is easy
if not action.handles_multiple:
response = action.single(self, self._meta.request, obj_id)
@@ -753,6 +757,12 @@ class DataTable(object):
return self.take_action(action, obj_id)
return None
def sanitize_id(self, obj_id):
""" Override to modify an incoming obj_id to match existing
API data types or modify the format.
"""
return obj_id
def get_object_id(self, datum):
""" Returns the identifier for the object this row will represent.
@@ -761,6 +771,14 @@ class DataTable(object):
"""
return datum.id
def get_object_display(self, datum):
""" Returns a display name that identifies this object.
By default, this returns a ``name`` attribute from the given object,
but this can be overriden to return other values.
"""
return datum.name
def has_more_data(self):
"""
Returns a boolean value indicating whether there is more data