Add pages menu to volume backups pagination

The backups pagination was with a bug that prevents users from
access older backups when the number of backups exceeds the
page size configurated in 'API_RESULT_PAGE_SIZE' properties.

The bug was occurring because the pages displays (Next, Prev) were
not displayed in the backups page, leaving the user with only
the first page to access their backups, so the backups from other
pages are unreachable.

Was fixed this bug and also was added new buttons to access
a specific, the last and the first pages. Easing the users'
navigation through the backups pages.

Change-Id: I148634152484f9718759775b81aab3703a296ef5

horizon/tables/__init__.py

@@ -31,6 +31,7 @@ from horizon.tables.views import MixedDataTableView
 from horizon.tables.views import MultiTableMixin
 from horizon.tables.views import MultiTableView
 from horizon.tables.views import PagedTableMixin
+from horizon.tables.views import PagedTableWithPageMenu
 
 
 __all__ = [
@@ -50,4 +51,5 @@ __all__ = [
     'MultiTableMixin',
     'MultiTableView',
     'PagedTableMixin',
+    'PagedTableWithPageMenu',
 ]

horizon/tables/views.py

@@ -389,3 +389,47 @@ class PagedTableMixin(object):
             if marker:
                 return marker, "desc"
             return None, "desc"
+
+
+class PagedTableWithPageMenu(object):
+    def __init__(self, *args, **kwargs):
+        super(PagedTableWithPageMenu, self).__init__(*args, **kwargs)
+        self._current_page = 1
+        self._number_of_pages = 0
+        self._total_of_entries = 0
+        self._page_size = 0
+
+    def handle_table(self, table):
+        name = table.name
+        self._tables[name]._meta.current_page = self.current_page
+        self._tables[name]._meta.number_of_pages = self.number_of_pages
+        return super(PagedTableWithPageMenu, self).handle_table(table)
+
+    def has_prev_data(self, table):
+        return self._current_page > 1
+
+    def has_more_data(self, table):
+        return self._current_page < self._number_of_pages
+
+    def current_page(self, table=None):
+        return self._current_page
+
+    def number_of_pages(self, table=None):
+        return self._number_of_pages
+
+    def current_offset(self, table):
+        return self._current_page * self._page_size + 1
+
+    def get_page_param(self, table):
+        try:
+            meta = self.table_class._meta
+        except AttributeError:
+            meta = self.table_classes[0]._meta
+
+        return meta.pagination_param
+
+    def _get_page_number(self):
+        page_number = self.request.GET.get(self.get_page_param(None), None)
+        if page_number:
+            return int(page_number)
+        return 1

horizon/templates/horizon/common/_data_table.html

@@ -24,7 +24,11 @@
       {% endif %}
   {% endblock table_breadcrumb %}
   {% if table.footer and rows %}
-    {% include "horizon/common/_data_table_pagination.html" %}
+    {% if table.number_of_pages is defined %}
+      {% include "horizon/common/_data_table_pagination.html" %}
+    {% else %}
+      {% include "horizon/common/_data_table_pagination_with_pages.html" %}
+    {% endif %}
   {% endif %}
   {% block table_columns %}
       {% if not table.is_browser_table %}
@@ -72,7 +76,11 @@
         {% endfor %}
       </tr>
       {% endif %}
-      {% include "horizon/common/_data_table_pagination.html" %}
+      {% if table.number_of_pages is defined %}
+        {% include "horizon/common/_data_table_pagination.html" %}
+      {% else %}
+        {% include "horizon/common/_data_table_pagination_with_pages.html" %}
+      {% endif %}
     </tfoot>
     {% endif %}
   {% endblock table_footer %}

horizon/templates/horizon/common/_data_table_pagination_with_pages.html

@@ -0,0 +1,27 @@
+{% load i18n %}
+{% load form_helpers %}
+<tr>
+    <td colspan="{{ columns|length }}">
+        <span class="table_count">{% blocktrans count counter=rows|length trimmed %}
+            Displaying {{ counter }} item{% plural %}
+            Displaying {{ counter }} items{% endblocktrans %}</span>
+        {% if table.has_prev_data or table.has_more_data %}
+            <span class="spacer">|</span>
+        {% endif %}
+        {% if table.has_prev_data %}
+            <a href="{{ table.get_pagination_string }}1">{% trans "&laquo;&laquo;&nbsp;First" %}</a>
+            <a href="{{ table.get_pagination_string }}{{ table.current_page | add:-1 }}">{% trans "&laquo;&nbsp;Prev&nbsp;" %}</a>
+        {% endif %}
+        {% for page in table.number_of_pages|get_range %}
+            {% if table.current_page == page %}
+                <span>{{page}}&nbsp;</span>
+            {% else %}
+                <a href="{{ table.get_pagination_string }}{{page}}">{{page}}&nbsp;</a>
+            {% endif %}
+        {% endfor %}
+        {% if table.has_more_data %}
+            <a href="{{ table.get_pagination_string }}{{ table.current_page | add:1 }}">{% trans "Next&nbsp;&raquo;" %}</a>
+            <a href="{{ table.get_pagination_string }}{{ table.number_of_pages }}">{% trans "Last&nbsp;&raquo;&raquo;" %}</a>
+        {% endif %}
+    </td>
+</tr>

horizon/templatetags/form_helpers.py

@@ -80,3 +80,10 @@ def wrapper_classes(field):
     if is_multiple_checkbox(field):
         classes.append('multiple-checkbox')
     return ' '.join(classes)
+
+
+@register.filter
+def get_range(val):
+    if val:
+        return range(1, val + 1)
+    return []

openstack_dashboard/api/cinder.py

@@ -21,6 +21,7 @@
 from __future__ import absolute_import
 
 import logging
+import math
 
 from django.conf import settings
 from django.utils.translation import pgettext_lazy
@@ -585,6 +586,42 @@ def volume_backup_list(request):
     return backups
 
 
+@profiler.trace
+def volume_backup_list_paged_with_page_menu(request, page_number=1,
+                                            sort_dir="desc"):
+    backups = []
+    count = 0
+    pages_count = 0
+    page_size = utils.get_page_size(request)
+    c_client = cinderclient(request, '3.45')
+
+    if c_client is None:
+        return backups, 0, count, pages_count
+
+    if VERSIONS.active > 1:
+        offset = (page_number - 1) * page_size
+        sort = 'created_at:' + sort_dir
+        bkps, count = c_client.backups.list(limit=page_size,
+                                            sort=sort,
+                                            search_opts={'with_count': True,
+                                                         'offset': offset})
+        if not bkps:
+            return backups, page_size, count, pages_count
+
+        if isinstance(bkps[0], list):
+            bkps = bkps[0]
+        pages_count = int(math.ceil(float(count) / float(page_size)))
+        for b in bkps:
+            backups.append(VolumeBackup(b))
+
+        return backups, page_size, count, pages_count
+    else:
+        for b in c_client.backups.list():
+            backups.append(VolumeBackup(b))
+
+    return backups, 0, count, pages_count
+
+
 @profiler.trace
 def volume_backup_list_paged(request, marker=None, paginate=False,
                              sort_dir="desc"):

openstack_dashboard/dashboards/project/backups/tables.py

@@ -178,11 +178,19 @@ class BackupsTable(tables.DataTable):
                               verbose_name=_("Snapshot"),
                               link="horizon:project:snapshots:detail")
 
+    def current_page(self):
+        return self._meta.current_page()
+
+    def number_of_pages(self):
+        return self._meta.number_of_pages()
+
+    def get_pagination_string(self):
+        return '?%s=' % self._meta.pagination_param
+
     class Meta(object):
         name = "volume_backups"
         verbose_name = _("Volume Backups")
-        pagination_param = 'backup_marker'
-        prev_pagination_param = 'prev_backup_marker'
+        pagination_param = 'page'
         status_columns = ("status",)
         row_class = UpdateRow
         table_actions = (DeleteBackup,)

openstack_dashboard/dashboards/project/backups/tests.py

@@ -28,11 +28,13 @@ INDEX_URL = reverse('horizon:project:backups:index')
 class VolumeBackupsViewTests(test.TestCase):
 
     @test.create_mocks({api.cinder: ('volume_list', 'volume_snapshot_list',
-                                     'volume_backup_list_paged')})
-    def _test_backups_index_paginated(self, marker, sort_dir, backups, url,
-                                      has_more, has_prev):
-        self.mock_volume_backup_list_paged.return_value = [backups,
-                                                           has_more, has_prev]
+                                     'volume_backup_list_paged_with_page_menu')
+                        })
+    def _test_backups_index_paginated(self, page_number, backups,
+                                      url, page_size, total_of_entries,
+                                      number_of_pages, has_prev, has_more):
+        self.mock_volume_backup_list_paged_with_page_menu.return_value = [
+            backups, page_size, total_of_entries, number_of_pages]
         self.mock_volume_list.return_value = self.cinder_volumes.list()
         self.mock_volume_snapshot_list.return_value \
             = self.cinder_volume_snapshots.list()
@@ -41,9 +43,17 @@ class VolumeBackupsViewTests(test.TestCase):
 
         self.assertEqual(res.status_code, 200)
         self.assertTemplateUsed(res, 'horizon/common/_data_table_view.html')
-        self.mock_volume_backup_list_paged.assert_called_once_with(
-            test.IsHttpRequest(), marker=marker, sort_dir=sort_dir,
-            paginate=True)
+        self.assertEqual(has_more,
+                         res.context_data['view'].has_more_data(None))
+        self.assertEqual(has_prev,
+                         res.context_data['view'].has_prev_data(None))
+        self.assertEqual(
+            page_number, res.context_data['view'].current_page(None))
+        self.assertEqual(
+            number_of_pages, res.context_data['view'].number_of_pages(None))
+        self.mock_volume_backup_list_paged_with_page_menu.\
+            assert_called_once_with(test.IsHttpRequest(),
+                                    page_number=page_number)
         self.mock_volume_list.assert_called_once_with(test.IsHttpRequest())
         self.mock_volume_snapshot_list.assert_called_once_with(
             test.IsHttpRequest())
@@ -55,34 +65,38 @@ class VolumeBackupsViewTests(test.TestCase):
         expected_snapshosts = self.cinder_volume_snapshots.list()
         size = settings.API_RESULT_PAGE_SIZE
         base_url = INDEX_URL
-        next = backup_tables.BackupsTable._meta.pagination_param
+        number_of_pages = len(backups)
+        pag = backup_tables.BackupsTable._meta.pagination_param
+        page_number = 1
 
         # get first page
         expected_backups = backups[:size]
         res = self._test_backups_index_paginated(
-            marker=None, sort_dir="desc", backups=expected_backups,
-            url=base_url, has_more=True, has_prev=False)
+            page_number=page_number, backups=expected_backups, url=base_url,
+            has_more=True, has_prev=False, page_size=size,
+            number_of_pages=number_of_pages, total_of_entries=number_of_pages)
         result = res.context['volume_backups_table'].data
         self.assertCountEqual(result, expected_backups)
 
         # get second page
         expected_backups = backups[size:2 * size]
-        marker = expected_backups[0].id
-
-        url = base_url + "?%s=%s" % (next, marker)
+        page_number = 2
+        url = base_url + "?%s=%s" % (pag, page_number)
         res = self._test_backups_index_paginated(
-            marker=marker, sort_dir="desc", backups=expected_backups, url=url,
-            has_more=True, has_prev=True)
+            page_number=page_number, backups=expected_backups, url=url,
+            has_more=True, has_prev=True, page_size=size,
+            number_of_pages=number_of_pages, total_of_entries=number_of_pages)
         result = res.context['volume_backups_table'].data
         self.assertCountEqual(result, expected_backups)
         self.assertEqual(result[0].snapshot.id, expected_snapshosts[1].id)
         # get last page
         expected_backups = backups[-size:]
-        marker = expected_backups[0].id
-        url = base_url + "?%s=%s" % (next, marker)
+        page_number = 3
+        url = base_url + "?%s=%s" % (pag, page_number)
         res = self._test_backups_index_paginated(
-            marker=marker, sort_dir="desc", backups=expected_backups, url=url,
-            has_more=False, has_prev=True)
+            page_number=page_number, backups=expected_backups, url=url,
+            has_more=False, has_prev=True, page_size=size,
+            number_of_pages=number_of_pages, total_of_entries=number_of_pages)
         result = res.context['volume_backups_table'].data
         self.assertCountEqual(result, expected_backups)
 
@@ -90,26 +104,29 @@ class VolumeBackupsViewTests(test.TestCase):
     def test_backups_index_paginated_prev_page(self):
         backups = self.cinder_volume_backups.list()
         size = settings.API_RESULT_PAGE_SIZE
+        number_of_pages = len(backups)
         base_url = INDEX_URL
-        prev = backup_tables.BackupsTable._meta.prev_pagination_param
+        pag = backup_tables.BackupsTable._meta.pagination_param
 
         # prev from some page
         expected_backups = backups[size:2 * size]
-        marker = expected_backups[0].id
-        url = base_url + "?%s=%s" % (prev, marker)
+        page_number = 2
+        url = base_url + "?%s=%s" % (pag, page_number)
         res = self._test_backups_index_paginated(
-            marker=marker, sort_dir="asc", backups=expected_backups, url=url,
-            has_more=True, has_prev=True)
+            page_number=page_number, backups=expected_backups, url=url,
+            has_more=True, has_prev=True, page_size=size,
+            number_of_pages=number_of_pages, total_of_entries=number_of_pages)
         result = res.context['volume_backups_table'].data
         self.assertCountEqual(result, expected_backups)
 
         # back to first page
         expected_backups = backups[:size]
-        marker = expected_backups[0].id
-        url = base_url + "?%s=%s" % (prev, marker)
+        page_number = 1
+        url = base_url + "?%s=%s" % (pag, page_number)
         res = self._test_backups_index_paginated(
-            marker=marker, sort_dir="asc", backups=expected_backups, url=url,
-            has_more=True, has_prev=False)
+            page_number=page_number, backups=expected_backups, url=url,
+            has_more=True, has_prev=False, page_size=size,
+            number_of_pages=number_of_pages, total_of_entries=number_of_pages)
         result = res.context['volume_backups_table'].data
         self.assertCountEqual(result, expected_backups)
 
@@ -267,16 +284,20 @@ class VolumeBackupsViewTests(test.TestCase):
 
     @test.create_mocks({api.cinder: ('volume_list',
                                      'volume_snapshot_list',
-                                     'volume_backup_list_paged',
+                                     'volume_backup_list_paged_with_page_menu',
                                      'volume_backup_delete')})
     def test_delete_volume_backup(self):
         vol_backups = self.cinder_volume_backups.list()
         volumes = self.cinder_volumes.list()
         backup = self.cinder_volume_backups.first()
         snapshots = self.cinder_volume_snapshots.list()
+        page_number = 1
+        page_size = 1
+        total_of_entries = 1
+        number_of_pages = 1
 
-        self.mock_volume_backup_list_paged.return_value = [vol_backups,
-                                                           False, False]
+        self.mock_volume_backup_list_paged_with_page_menu.return_value = [
+            vol_backups, page_size, total_of_entries, number_of_pages]
         self.mock_volume_list.return_value = volumes
         self.mock_volume_backup_delete.return_value = None
         self.mock_volume_snapshot_list.return_value = snapshots
@@ -286,9 +307,9 @@ class VolumeBackupsViewTests(test.TestCase):
 
         self.assertRedirectsNoFollow(res, INDEX_URL)
         self.assertMessageCount(success=1)
-        self.mock_volume_backup_list_paged.assert_called_once_with(
-            test.IsHttpRequest(), marker=None, sort_dir='desc',
-            paginate=True)
+        self.mock_volume_backup_list_paged_with_page_menu.\
+            assert_called_once_with(test.IsHttpRequest(),
+                                    page_number=page_number)
         self.mock_volume_list.assert_called_once_with(test.IsHttpRequest())
         self.mock_volume_snapshot_list.assert_called_once_with(
             test.IsHttpRequest())

openstack_dashboard/dashboards/project/backups/views.py

@@ -10,6 +10,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+import logging
+
 from django.urls import reverse
 from django.urls import reverse_lazy
 from django.utils.translation import ugettext_lazy as _
@@ -30,8 +32,10 @@ from openstack_dashboard.dashboards.project.backups \
 from openstack_dashboard.dashboards.project.volumes \
     import views as volume_views
 
+LOG = logging.getLogger(__name__)
 
-class BackupsView(tables.DataTableView, tables.PagedTableMixin,
+
+class BackupsView(tables.PagedTableWithPageMenu, tables.DataTableView,
                   volume_views.VolumeTableMixIn):
     table_class = backup_tables.BackupsTable
     page_title = _("Volume Backups")
@@ -41,11 +45,11 @@ class BackupsView(tables.DataTableView, tables.PagedTableMixin,
 
     def get_data(self):
         try:
-            marker, sort_dir = self._get_marker()
-            backups, self._has_more_data, self._has_prev_data = \
-                api.cinder.volume_backup_list_paged(
-                    self.request, marker=marker, sort_dir=sort_dir,
-                    paginate=True)
+            self._current_page = self._get_page_number()
+            (backups, self._page_size, self._total_of_entries,
+             self._number_of_pages) = \
+                api.cinder.volume_backup_list_paged_with_page_menu(
+                    self.request, page_number=self._current_page)
             volumes = api.cinder.volume_list(self.request)
             volumes = dict((v.id, v) for v in volumes)
             snapshots = api.cinder.volume_snapshot_list(self.request)
@@ -53,7 +57,8 @@ class BackupsView(tables.DataTableView, tables.PagedTableMixin,
             for backup in backups:
                 backup.volume = volumes.get(backup.volume_id)
                 backup.snapshot = snapshots.get(backup.snapshot_id)
-        except Exception:
+        except Exception as e:
+            LOG.exception(e)
             backups = []
             exceptions.handle(self.request, _("Unable to retrieve "
                                               "volume backups."))