Fixing Nameless Volume Display

When a volume has no name, it now shows the
ID instead. I also added a fixture data that
represents such a volume being in the system.

Fixes bug #1012380

You can also now use truncate=i on DataTable
Columns in order to have the value truncated if
it goes beyond i characters.

On top of this I fixed/made-clearer a bunch of
the tests relating to volumes and instances.
Please do NOT use deepcopy() of fixture data
in tests any more, instead just create a new
fixture for the data you want.

Change-Id: If2f92b5d04b04f08f5cacca03f614fce5ea38702

horizon/dashboards/nova/instances_and_volumes/tests.py

@@ -18,7 +18,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from copy import deepcopy
 from django import http
 from django.core.urlresolvers import reverse
 from mox import IsA
@@ -28,10 +27,8 @@
 
 
 class InstancesAndVolumesViewTest(test.TestCase):
+    @test.create_stubs({api: ('flavor_list', 'server_list', 'volume_list',)})
     def test_index(self):
-        self.mox.StubOutWithMock(api, 'flavor_list')
-        self.mox.StubOutWithMock(api, 'server_list')
-        self.mox.StubOutWithMock(api, 'volume_list')
         api.flavor_list(IsA(http.HttpRequest)).AndReturn(self.flavors.list())
         api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())
         api.volume_list(IsA(http.HttpRequest)).AndReturn(self.volumes.list())
@@ -49,23 +46,12 @@ def test_index(self):
         self.assertItemsEqual(instances, self.servers.list())
         self.assertItemsEqual(volumes, self.volumes.list())
 
+    @test.create_stubs({api: ('flavor_list', 'server_list', 'volume_list',)})
     def test_attached_volume(self):
-        volumes = deepcopy(self.volumes.list())
-        attached_volume = deepcopy(self.volumes.list()[0])
-        attached_volume.id = "2"
-        attached_volume.display_name = "Volume2 name"
-        attached_volume.size = "80"
-        attached_volume.status = "in-use"
-        attached_volume.attachments = [{"server_id": "1",
-                                        "device": "/dev/hdn"}]
-        volumes.append(attached_volume)
-
-        self.mox.StubOutWithMock(api, 'server_list')
-        self.mox.StubOutWithMock(api, 'volume_list')
-        self.mox.StubOutWithMock(api, 'flavor_list')
         api.flavor_list(IsA(http.HttpRequest)).AndReturn(self.flavors.list())
         api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())
-        api.volume_list(IsA(http.HttpRequest)).AndReturn(volumes)
+        api.volume_list(IsA(http.HttpRequest)) \
+                .AndReturn(self.volumes.list()[1:3])
 
         self.mox.ReplayAll()
 
@@ -78,21 +64,20 @@ def test_attached_volume(self):
         resp_volumes = res.context['volumes_table'].data
 
         self.assertItemsEqual(instances, self.servers.list())
-        self.assertItemsEqual(resp_volumes, volumes)
-
-        self.assertContains(res, ">Volume name<", 1, 200)
-        self.assertContains(res, ">40GB<", 1, 200)
-        self.assertContains(res, ">Available<", 1, 200)
-
-        self.assertContains(res, ">Volume2 name<", 1, 200)
-        self.assertContains(res, ">80GB<", 1, 200)
-        self.assertContains(res, ">In-Use<", 1, 200)
-        self.assertContains(res, ">server_1<", 2, 200)
-        self.assertContains(res, "on /dev/hdn", 1, 200)
-
+        self.assertItemsEqual(resp_volumes, self.volumes.list()[1:3])
+
+        self.assertContains(res, ">My Volume<", 1, 200)
+        self.assertContains(res, ">30GB<", 1, 200)
+        self.assertContains(res, ">3b189ac8-9166-ac7f-90c9-16c8bf9e01ac<",
+                            1,
+                            200)
+        self.assertContains(res, ">10GB<", 1, 200)
+        self.assertContains(res, ">In-Use<", 2, 200)
+        self.assertContains(res, "on /dev/hda", 1, 200)
+        self.assertContains(res, "on /dev/hdk", 1, 200)
+
+    @test.create_stubs({api: ('server_list', 'volume_list',)})
     def test_index_server_list_exception(self):
-        self.mox.StubOutWithMock(api, 'server_list')
-        self.mox.StubOutWithMock(api, 'volume_list')
         api.server_list(IsA(http.HttpRequest)).AndRaise(self.exceptions.nova)
         api.volume_list(IsA(http.HttpRequest)).AndReturn(self.volumes.list())
         api.server_list(IsA(http.HttpRequest)).AndReturn(self.servers.list())

horizon/dashboards/nova/instances_and_volumes/views.py

@@ -22,7 +22,6 @@
 """
 Views for Instances and Volumes.
 """
-import re
 import logging
 
 from django.utils.translation import ugettext_lazy as _
@@ -73,16 +72,12 @@ def get_volumes_data(self):
             instances = SortedDict([(inst.id, inst) for inst in
                                     self._get_instances()])
             for volume in volumes:
-                # Truncate the description for proper display.
-                if len(getattr(volume, 'display_description', '')) > 33:
-                    truncated_string = volume.display_description[:30].strip()
-                    # Remove non-word, and underscore characters, from the end
-                    # of the string before we add the ellepsis.
-                    truncated_string = re.sub(ur'[^\w\s]+$',
-                                              '',
-                                              truncated_string)
+                # It is possible to create a volume with no name through the
+                # EC2 API, use the ID in those cases.
+                if not volume.display_name:
+                    volume.display_name = volume.id
 
-                    volume.display_description = truncated_string + u'...'
+                description = getattr(volume, 'display_description', '')
 
                 for att in volume.attachments:
                     server_id = att.get('server_id', None)

horizon/dashboards/nova/instances_and_volumes/volumes/tables.py

@@ -135,7 +135,8 @@ class VolumesTableBase(tables.DataTable):
     name = tables.Column("display_name", verbose_name=_("Name"),
                          link="%s:volumes:detail" % URL_PREFIX)
     description = tables.Column("display_description",
-                                verbose_name=_("Description"))
+                                verbose_name=_("Description"),
+                                truncate=40)
     size = tables.Column(get_size, verbose_name=_("Size"))
     status = tables.Column("status",
                            filters=(title,),

horizon/dashboards/nova/instances_and_volumes/volumes/tests.py

@@ -18,7 +18,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-from copy import deepcopy
 from django import http
 from django.core.urlresolvers import reverse
 from mox import IsA
@@ -28,14 +27,15 @@
 
 
 class VolumeViewTests(test.TestCase):
+    @test.create_stubs({api: ('volume_get',), api.nova: ('server_list',)})
     def test_edit_attachments(self):
         volume = self.volumes.first()
         servers = self.servers.list()
-        self.mox.StubOutWithMock(api, 'volume_get')
-        self.mox.StubOutWithMock(api.nova, 'server_list')
+
         api.volume_get(IsA(http.HttpRequest), volume.id) \
                        .AndReturn(volume)
         api.nova.server_list(IsA(http.HttpRequest)).AndReturn(servers)
+
         self.mox.ReplayAll()
 
         url = reverse('horizon:nova:instances_and_volumes:volumes:attach',
@@ -47,62 +47,51 @@ def test_edit_attachments(self):
                          2)
         self.assertEqual(res.status_code, 200)
 
+    @test.create_stubs({api: ('volume_get',),
+                        api.nova: ('server_get', 'server_list',)})
     def test_edit_attachments_attached_volume(self):
         server = self.servers.first()
-        servers = deepcopy(self.servers)
-        active_server = deepcopy(self.servers.first())
-        active_server.status = 'ACTIVE'
-        active_server.id = "3"
-        servers.add(active_server)
-        volumes = deepcopy(self.volumes)
-        volume = deepcopy(self.volumes.first())
-        volume.id = "2"
-        volume.status = "in-use"
-        volume.attachments = [{"id": "1", "server_id": server.id,
-                               "device": "/dev/hdn"}]
-        volumes.add(volume)
-
-        self.mox.StubOutWithMock(api, 'volume_get')
-        self.mox.StubOutWithMock(api.nova, 'server_list')
-        self.mox.StubOutWithMock(api.nova, 'server_get')
-        api.nova.server_get(IsA(http.HttpRequest), server.id).AndReturn(server)
+        volume = self.volumes.list()[0]
+
         api.volume_get(IsA(http.HttpRequest), volume.id) \
                        .AndReturn(volume)
-        api.nova.server_list(IsA(http.HttpRequest)).AndReturn(servers.list())
+        api.nova.server_list(IsA(http.HttpRequest)) \
+                             .AndReturn(self.servers.list())
+
         self.mox.ReplayAll()
 
         url = reverse('horizon:nova:instances_and_volumes:volumes:attach',
                       args=[volume.id])
         res = self.client.get(url)
-        # Asserting length of 1 instance (plus 'Select ..' item).
-        # The other instance is already attached to this volume
-        self.assertEqual(len(res.context['form'].fields['instance']._choices),
-                         2)
+
         self.assertEqual(res.context['form'].\
                                 fields['instance']._choices[0][1],
                                 "Select an instance")
-        # The instance choice should not be server_id = 3
-        self.assertNotEqual(res.context['form'].\
-                                fields['instance']._choices[1][0],
-                                volume.attachments[0]['server_id'])
+        self.assertEqual(len(res.context['form'].fields['instance'].choices),
+                         2)
+        self.assertEqual(res.context['form'].fields['instance']._choices[1][0],
+                         server.id)
         self.assertEqual(res.status_code, 200)
 
+    @test.create_stubs({api.nova: ('volume_get', 'server_get',)})
     def test_detail_view(self):
         volume = self.volumes.first()
         server = self.servers.first()
+
         volume.attachments = [{"server_id": server.id}]
-        self.mox.StubOutWithMock(api.nova, 'volume_get')
-        self.mox.StubOutWithMock(api.nova, 'server_get')
+
         api.nova.volume_get(IsA(http.HttpRequest), volume.id).AndReturn(volume)
         api.nova.server_get(IsA(http.HttpRequest), server.id).AndReturn(server)
+
         self.mox.ReplayAll()
 
         url = reverse('horizon:nova:instances_and_volumes:volumes:detail',
                       args=[volume.id])
         res = self.client.get(url)
 
         self.assertContains(res, "<dd>Volume name</dd>", 1, 200)
-        self.assertContains(res, "<dd>1</dd>", 1, 200)
+        self.assertContains(res, "<dd>41023e92-8008-4c8b-8059-" \
+                                 "7f2293ff3775</dd>", 1, 200)
         self.assertContains(res, "<dd>Available</dd>", 1, 200)
         self.assertContains(res, "<dd>40 GB</dd>", 1, 200)
         self.assertContains(res, "<dd>04/01/12 at 10:30:00</dd>", 1, 200)

horizon/tables/base.py

@@ -26,6 +26,7 @@
 from django.conf import settings
 from django.contrib import messages
 from django.core import urlresolvers
+from django.template.defaultfilters import truncatechars
 from django.template.loader import render_to_string
 from django.utils import http
 from django.utils.datastructures import SortedDict
@@ -142,6 +143,14 @@ class Column(html.HTMLElement):
 
         A dict of HTML attribute strings which should be added to this column.
         Example: ``attrs={"data-foo": "bar"}``.
+
+    .. attribute:: truncate
+
+        An integer for the maximum length of the string in this column. If the
+        data in this column is larger than the supplied number, the data for
+        this column will be truncated and an ellipsis will be appended to the
+        truncated data.
+        Defaults to ``None``.
     """
     summation_methods = {
         "sum": sum,
@@ -172,31 +181,35 @@ class Column(html.HTMLElement):
     def __init__(self, transform, verbose_name=None, sortable=True,
                  link=None, hidden=False, attrs=None, status=False,
                  status_choices=None, display_choices=None, empty_value=None,
-                 filters=None, classes=None, summation=None, auto=None):
+                 filters=None, classes=None, summation=None, auto=None,
+                 truncate=None):
         self.classes = list(classes or getattr(self, "classes", []))
         super(Column, self).__init__()
         self.attrs.update(attrs or {})
 
-        self.auto = auto
-
         if callable(transform):
             self.transform = transform
             self.name = transform.__name__
         else:
             self.transform = unicode(transform)
             self.name = self.transform
-        self.sortable = sortable
+
         # Empty string is a valid value for verbose_name
         if verbose_name is None:
             verbose_name = self.transform.title()
         else:
             verbose_name = verbose_name
+
+        self.auto = auto
+        self.sortable = sortable
         self.verbose_name = verbose_name
         self.link = link
         self.hidden = hidden
         self.status = status
         self.empty_value = empty_value or '-'
         self.filters = filters or []
+        self.truncate = truncate
+
         if status_choices:
             self.status_choices = status_choices
         self.display_choices = display_choices
@@ -257,20 +270,29 @@ def get_data(self, datum):
         method for this column.
         """
         datum_id = self.table.get_object_id(datum)
+
         if datum_id in self.table._data_cache[self]:
             return self.table._data_cache[self][datum_id]
+
         data = self.get_raw_data(datum)
         display_value = None
+
         if self.display_choices:
             display_value = [display for (value, display) in
                              self.display_choices
                              if value.lower() == (data or '').lower()]
+
         if display_value:
             data = display_value[0]
         else:
             for filter_func in self.filters:
                 data = filter_func(data)
+
+        if data and self.truncate:
+            data = truncatechars(data, self.truncate)
+
         self.table._data_cache[self][datum_id] = data
+
         return self.table._data_cache[self][datum_id]
 
     def get_link_url(self, datum):

horizon/tests/table_tests.py

@@ -56,6 +56,11 @@ def __repr__(self):
     FakeObject('2', 'object_2', 4, 'up'),
 )
 
+TEST_DATA_5 = (
+    FakeObject('1', 'object_1', 'A Value That is longer than 35 characters!',
+               'down', 'optional_1'),
+)
+
 
 class MyLinkAction(tables.LinkAction):
     name = "login"
@@ -153,7 +158,8 @@ class MyTable(tables.DataTable):
                           sortable=True,
                           link='http://example.com/',
                           attrs={'class': 'green blue'},
-                          summation="average")
+                          summation="average",
+                          truncate=35)
     status = tables.Column('status', link=get_link)
     optional = tables.Column('optional', empty_value='N/A')
     excluded = tables.Column('excluded')
@@ -379,6 +385,14 @@ def test_table_row(self):
         self.assertEqual(row.cells['status'].get_status_class(cell_status),
                          'status_up')
 
+    def test_table_column_truncation(self):
+        self.table = MyTable(self.request, TEST_DATA_5)
+        row = self.table.get_rows()[0]
+
+        self.assertEqual(len(row.cells['value'].data), 35)
+        self.assertEqual(row.cells['value'].data,
+                         u'A Value That is longer than 35 c...')
+
     def test_table_rendering(self):
         self.table = MyTable(self.request, TEST_DATA)
         # Table actions

horizon/tests/test_data/nova_data.py

@@ -146,14 +146,38 @@ def data(TEST):
 
     # Volumes
     volume = volumes.Volume(volumes.VolumeManager(None),
-                            dict(id="1",
+                            dict(id="41023e92-8008-4c8b-8059-7f2293ff3775",
                                  name='test_volume',
                                  status='available',
                                  size=40,
                                  display_name='Volume name',
                                  created_at='2012-04-01 10:30:00',
-                                 attachments={}))
+                                 attachments=[]))
+    nameless_volume = volumes.Volume(volumes.VolumeManager(None),
+                         dict(id="3b189ac8-9166-ac7f-90c9-16c8bf9e01ac",
+                              name='',
+                              status='in-use',
+                              size=10,
+                              display_name='',
+                              display_description='',
+                              device="/dev/hda",
+                              created_at='2010-11-21 18:34:25',
+                              attachments=[{"id": "1", "server_id": '1',
+                                            "device": "/dev/hda"}]))
+    attached_volume = volumes.Volume(volumes.VolumeManager(None),
+                         dict(id="8cba67c1-2741-6c79-5ab6-9c2bf8c96ab0",
+                              name='my_volume',
+                              status='in-use',
+                              size=30,
+                              display_name='My Volume',
+                              display_description='',
+                              device="/dev/hdk",
+                              created_at='2011-05-01 11:54:33',
+                              attachments=[{"id": "2", "server_id": '1',
+                                            "device": "/dev/hdk"}]))
     TEST.volumes.add(volume)
+    TEST.volumes.add(nameless_volume)
+    TEST.volumes.add(attached_volume)
 
     # Flavors
     flavor_1 = flavors.Flavor(flavors.FlavorManager(None),