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

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_present = (_("Pause"), _("Unpause"))
-    action_past = _("Paused")
+    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.current_present_action = UNPAUSE
-            self.action_past = _("Unpaused")
+        return instance.status in ACTIVE_STATES or self.paused
-        return instance.status in ACTIVE_STATES
 
     def action(self, request, obj_id):
-        if getattr(self, 'paused', False):
+        if self.paused:
-            api.server_pause(request, obj_id)
-        else:
             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_present = (_("Suspend"), _("Resume"))
-    action_past = _("Suspended")
+    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.current_present_action = RESUME
-            self.action_past = _("Resumed")
+        return instance.status in ACTIVE_STATES or self.suspended
-        return instance.status in ACTIVE_STATES
 
     def action(self, request, obj_id):
-        if getattr(self, 'suspended', False):
+        if self.suspended:
-            api.server_suspend(request, obj_id)
-        else:
             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):

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()
+        # Priority: constructor, class-defined, fallback
-        self.verbose_name = unicode(getattr(self,
+        self.verbose_name = verbose_name or getattr(self, 'verbose_name',
-                                            "verbose_name",
+                                                    self.name.title())
-                                            verbose_name))
+        self.verbose_name_plural = verbose_name_plural or \
-        verbose_name_plural = verbose_name_plural or "%ss" % self.verbose_name
+                                    getattr(self, 'verbose_name_plural',
-        self.verbose_name_plural = unicode(getattr(self,
+                                           "%ss" % self.verbose_name)
-                                                   "verbose_name_plural",
-                                                   verbose_name_plural))
         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 = verbose_name or unicode(getattr(self,
-        self.verbose_name = 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,
+       String or tuple/list. The display forms of the name.
-       capitalized and translated. ("Delete", "Rotate", etc.)
+       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))

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):
                                  ['<MyAction: delete>',
                                   '<MyFilterAction: filter>',
                                   '<MyLinkAction: login>',
+                                  '<MyBatchAction: toggle>',
                                   '<MyUpdateAction: update>'])
         self.assertQuerysetEqual(self.table.get_table_actions(),
                                  ['<MyFilterAction: filter>',
@@ -151,7 +177,8 @@ class DataTableTests(test.TestCase):
         self.assertQuerysetEqual(self.table.get_row_actions(TEST_DATA[0]),
                                  ['<MyAction: delete>',
                                   '<MyLinkAction: login>',
-                                  '<MyUpdateAction: update>'])
+                                  '<MyUpdateAction: update>',
+                                  '<MyBatchAction: toggle>'])
         # 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, "<li", 2)
+        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)
         # Whole table
@@ -363,6 +391,47 @@ class DataTableTests(test.TestCase):
         self.assertEqual(handled.status_code, 302)
         self.assertEqual(handled["location"], "http://example.com/1")
 
+        # Single object batch 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])), 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'
+        # POST page
+        action_string = "my_table__toggle__1"
+        req = self.factory.post('/my_url/', {'action': action_string})
+        self.table = MyTable(req, TEST_DATA)
+        self.assertEqual(self.table.parse_action(action_string),
+                         ('my_table', 'toggle', '1'))
+        handled = self.table.maybe_handle()
+        self.assertEqual(handled.status_code, 302)
+        self.assertEqual(handled["location"], "/my_url/")
+        self.assertEqual(list(req._messages)[0].message,
+                        u"Downed Item: object_1")
+
+        # Toggle from status 'down' to 'up'
+        # 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])), 3)
+        toggle_action = self.table.get_row_actions(TEST_DATA_2[0])[2]
+        self.assertEqual(unicode(toggle_action.verbose_name), "Up Item")
+
+        # POST page
+        action_string = "my_table__toggle__2"
+        req = self.factory.post('/my_url/', {'action': action_string})
+        self.table = MyTable(req, TEST_DATA)
+        self.assertEqual(self.table.parse_action(action_string),
+                         ('my_table', 'toggle', '2'))
+        handled = self.table.maybe_handle()
+        self.assertEqual(handled.status_code, 302)
+        self.assertEqual(handled["location"], "/my_url/")
+        self.assertEqual(list(req._messages)[0].message,
+                        u"Upped Item: object_2")
+
         # Multiple object action
         action_string = "my_table__delete"
         req = self.factory.post('/my_url/', {'action': action_string,
@@ -414,3 +483,12 @@ class DataTableTests(test.TestCase):
         self.assertEqual(resp, None)
         resp = self.table.maybe_handle()
         self.assertEqual(resp, None)
+
+        # Verbose names
+        table_actions = self.table.get_table_actions()
+        self.assertEqual(unicode(table_actions[0].verbose_name), "filter")
+        self.assertEqual(unicode(table_actions[1].verbose_name), "Delete Me")
+
+        row_actions = self.table.get_row_actions(TEST_DATA[0])
+        self.assertEqual(unicode(row_actions[0].verbose_name), "Delete Me")
+        self.assertEqual(unicode(row_actions[1].verbose_name), "Log In")