From 7369311aad6c22c6dbdf77c6a6f6a3c6e09c98ae Mon Sep 17 00:00:00 2001 From: Tihomir Trifonov Date: Fri, 3 Feb 2012 01:45:08 +0200 Subject: [PATCH] Fixes logic for toggle Pause/Suspend actions Fixes bug 925395. Added functionality in BatchAction to support multiple actions. The verbose_names are accordingly changed in update method. It is only required that the current action index is set in the control. Patch 2: Applied code changes to newly added classes Patch 3: Refactored logic, added unit tests for batch actions and for verbose_names on all actions Patch 4: Resolved conflicts with newly added code in horizon/horizon/tests/table_tests.py Change-Id: I7cbdce25b8ae027ff1a153b3379add1b66b8aa76 --- .../instances_and_volumes/instances/tables.py | 45 ++++++---- horizon/horizon/tables/actions.py | 71 ++++++++++------ horizon/horizon/tests/table_tests.py | 84 ++++++++++++++++++- 3 files changed, 153 insertions(+), 47 deletions(-) diff --git a/horizon/horizon/dashboards/nova/instances_and_volumes/instances/tables.py b/horizon/horizon/dashboards/nova/instances_and_volumes/instances/tables.py index 17e3908b1..858aeb321 100644 --- a/horizon/horizon/dashboards/nova/instances_and_volumes/instances/tables.py +++ b/horizon/horizon/dashboards/nova/instances_and_volumes/instances/tables.py @@ -43,6 +43,11 @@ POWER_STATES = { 9: "BUILDING", } +PAUSE = 0 +UNPAUSE = 1 +SUSPEND = 0 +RESUME = 1 + class TerminateInstance(tables.BatchAction): name = "terminate" @@ -73,48 +78,52 @@ class RebootInstance(tables.BatchAction): class TogglePause(tables.BatchAction): name = "pause" - action_present = _("Pause") - action_past = _("Paused") + action_present = (_("Pause"), _("Unpause")) + action_past = (_("Paused"), _("Unpaused")) data_type_singular = _("Instance") data_type_plural = _("Instances") def allowed(self, request, instance=None): + self.paused = False if not instance: - return True + return self.paused self.paused = instance.status == "PAUSED" if self.paused: - self.action_present = _("Unpause") - self.action_past = _("Unpaused") - return instance.status in ACTIVE_STATES + self.current_present_action = UNPAUSE + return instance.status in ACTIVE_STATES or self.paused def action(self, request, obj_id): - if getattr(self, 'paused', False): - api.server_pause(request, obj_id) - else: + if self.paused: api.server_unpause(request, obj_id) + self.current_past_action = UNPAUSE + else: + api.server_pause(request, obj_id) + self.current_past_action = PAUSE class ToggleSuspend(tables.BatchAction): name = "suspend" - action_present = _("Suspend") - action_past = _("Suspended") + action_present = (_("Suspend"), _("Resume")) + action_past = (_("Suspended"), _("Resumed")) data_type_singular = _("Instance") data_type_plural = _("Instances") def allowed(self, request, instance=None): + self.suspended = False if not instance: - return True + self.suspended self.suspended = instance.status == "SUSPENDED" if self.suspended: - self.action_present = _("Resume") - self.action_past = _("Resumed") - return instance.status in ACTIVE_STATES + self.current_present_action = RESUME + return instance.status in ACTIVE_STATES or self.suspended def action(self, request, obj_id): - if getattr(self, 'suspended', False): - api.server_suspend(request, obj_id) - else: + if self.suspended: api.server_resume(request, obj_id) + self.current_past_action = RESUME + else: + api.server_suspend(request, obj_id) + self.current_past_action = SUSPEND class LaunchLink(tables.LinkAction): diff --git a/horizon/horizon/tables/actions.py b/horizon/horizon/tables/actions.py index c62591178..45efa2ef1 100644 --- a/horizon/horizon/tables/actions.py +++ b/horizon/horizon/tables/actions.py @@ -149,14 +149,12 @@ class Action(BaseAction): single_func=None, multiple_func=None, handle_func=None, handles_multiple=False, attrs=None, requires_input=True): super(Action, self).__init__() - verbose_name = verbose_name or self.name.title() - self.verbose_name = unicode(getattr(self, - "verbose_name", - verbose_name)) - verbose_name_plural = verbose_name_plural or "%ss" % self.verbose_name - self.verbose_name_plural = unicode(getattr(self, - "verbose_name_plural", - verbose_name_plural)) + # Priority: constructor, class-defined, fallback + self.verbose_name = verbose_name or getattr(self, 'verbose_name', + self.name.title()) + self.verbose_name_plural = verbose_name_plural or \ + getattr(self, 'verbose_name_plural', + "%ss" % self.verbose_name) self.handles_multiple = getattr(self, "handles_multiple", handles_multiple) @@ -230,10 +228,9 @@ class LinkAction(BaseAction): def __init__(self, verbose_name=None, url=None, attrs=None): super(LinkAction, self).__init__() - verbose_name = verbose_name or self.name.title() - self.verbose_name = unicode(getattr(self, + self.verbose_name = verbose_name or unicode(getattr(self, "verbose_name", - verbose_name)) + self.name.title())) self.url = getattr(self, "url", url) if not self.verbose_name: raise NotImplementedError('A LinkAction object must have a ' @@ -354,7 +351,7 @@ class FilterAction(BaseAction): def __init__(self, verbose_name=None, param_name=None): super(FilterAction, self).__init__() - self.verbose_name = unicode(verbose_name) or self.name + self.verbose_name = unicode(verbose_name or self.name) self.param_name = param_name or 'q' def get_param_name(self): @@ -386,12 +383,18 @@ class BatchAction(Action): .. attribute:: action_present - The display form of the name. Should be a transitive verb, - capitalized and translated. ("Delete", "Rotate", etc.) + String or tuple/list. The display forms of the name. + Should be a transitive verb, capitalized and translated. ("Delete", + "Rotate", etc.) If tuple or list - then setting + self.current_present_action = n will set the current active item + from the list(action_present[n]) .. attribute:: action_past - The past tense of action_present. ("Deleted", "Rotated", etc.) + String or tuple/list. The past tense of action_present. ("Deleted", + "Rotated", etc.) If tuple or list - then + setting self.current_past_action = n will set the current active item + from the list(action_past[n]) .. attribute:: data_type_singular @@ -411,30 +414,36 @@ class BatchAction(Action): """ completion_url = None + def __init__(self): + self.current_present_action = 0 + self.current_past_action = 0 + 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 _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 + action = self.action_past \ + if isinstance(self.action_past, basestring) \ + else self.action_past[self.current_past_action] else: - action = self.action_present + action = self.action_present \ + if isinstance(self.action_present, basestring) \ + else self.action_present[self.current_present_action] 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): """ Required. Accepts a single object id and performs the specific action. @@ -444,6 +453,14 @@ class BatchAction(Action): raise NotImplementedError('action() must be defined for ' 'BatchAction: %s' % self.data_type_singular) + def update(self, request, datum): + """ + Switches the action verbose name, if needed + """ + if getattr(self, 'action_present', False): + self.verbose_name = self._conjugate() + self.verbose_name_plural = self._conjugate('plural') + def get_success_url(self, request=None): """ Returns the URL to redirect to after a successful action. @@ -466,6 +483,8 @@ class BatchAction(Action): continue try: self.action(request, datum_id) + #Call update to invoke changes if needed + self.update(request, datum) action_success.append(datum_display) LOG.info('%s: "%s"' % (self._conjugate(past=True), datum_display)) diff --git a/horizon/horizon/tests/table_tests.py b/horizon/horizon/tests/table_tests.py index 3eaf8f762..04851657a 100644 --- a/horizon/horizon/tests/table_tests.py +++ b/horizon/horizon/tests/table_tests.py @@ -46,6 +46,10 @@ TEST_DATA_2 = ( FakeObject('1', 'object_1', 'value_1', 'down', 'optional_1', 'excluded_1'), ) +TEST_DATA_3 = ( + FakeObject('1', 'object_1', 'value_1', 'up', 'optional_1', 'excluded_1'), +) + class MyLinkAction(tables.LinkAction): name = "login" @@ -76,6 +80,27 @@ class MyUpdateAction(tables.UpdateAction): return TEST_DATA_2[0] +class MyBatchAction(tables.BatchAction): + name = "toggle" + action_present = ("Down", "Up") + action_past = ("Downed", "Upped") + data_type_singular = _("Item") + data_type_plural = _("Items") + + def allowed(self, request, obj=None): + if not obj: + return False + self.down = getattr(obj, 'status', None) == 'down' + if self.down: + self.current_present_action = 1 + return self.down or getattr(obj, 'status', None) == 'up' + + def action(self, request, object_ids): + if self.down: + #up it + self.current_past_action = 1 + + class MyFilterAction(tables.FilterAction): def filter(self, table, objs, filter_string): q = filter_string.lower() @@ -113,7 +138,7 @@ class MyTable(tables.DataTable): status_column = "status" columns = ('id', 'name', 'value', 'optional', 'status') table_actions = (MyFilterAction, MyAction,) - row_actions = (MyAction, MyLinkAction, MyUpdateAction) + row_actions = (MyAction, MyLinkAction, MyUpdateAction, MyBatchAction,) class DataTableTests(test.TestCase): @@ -144,6 +169,7 @@ class DataTableTests(test.TestCase): ['', '', '', + '', '']) self.assertQuerysetEqual(self.table.get_table_actions(), ['', @@ -151,7 +177,8 @@ class DataTableTests(test.TestCase): self.assertQuerysetEqual(self.table.get_row_actions(TEST_DATA[0]), ['', '', - '']) + '', + '']) # Auto-generated columns multi_select = self.table.columns['multi_select'] self.assertEqual(multi_select.auto, "multi_select") @@ -329,11 +356,12 @@ class DataTableTests(test.TestCase): # Row actions row_actions = self.table.render_row_actions(TEST_DATA[0]) resp = http.HttpResponse(row_actions) - self.assertContains(resp, "