launchpad-reviewers team mailing list archive

[Martin Packman <martin.packman@xxxxxxxxxxxxx>]

Martin Packman has proposed merging lp:~gz/maas/1.2_backport_tag_view_pagination into lp:maas/1.2.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~gz/maas/1.2_backport_tag_view_pagination/+merge/133221

Backport the second pagination branch, which splits up the node lists on the tag page. This is less important that the node page which is already in the quantal branch, but completes the story.

For the original merge proposal, see:

<https://code.launchpad.net/~gz/maas/tag_view_pagination/+merge/130322>

-- 
https://code.launchpad.net/~gz/maas/1.2_backport_tag_view_pagination/+merge/133221
Your team MAAS Maintainers is requested to review the proposed merge of lp:~gz/maas/1.2_backport_tag_view_pagination into lp:maas/1.2.

=== modified file 'src/maasserver/templates/maasserver/tag_view.html'
--- src/maasserver/templates/maasserver/tag_view.html	2012-10-03 03:59:53 +0000
+++ src/maasserver/templates/maasserver/tag_view.html	2012-11-07 11:44:25 +0000
@@ -33,8 +33,9 @@
     {% endif %}
   </ul>
   <div id="nodes" class="block first pad-top">
-    <h2 class="block first line-top pad-top">Nodes</h2>
+    <h2 class="block first line-top pad-top">{{ paginator.count }} Node{{ paginator.count|pluralize }}</h2>
     {% include "maasserver/nodes_listing.html" %}
+    {% include "maasserver/pagination.html" %}
   </div>
 {% endblock %}
 

=== modified file 'src/maasserver/tests/test_views_tags.py'
--- src/maasserver/tests/test_views_tags.py	2012-10-10 11:29:26 +0000
+++ src/maasserver/tests/test_views_tags.py	2012-11-07 11:44:25 +0000
@@ -13,6 +13,7 @@
 __all__ = []
 
 from django.core.urlresolvers import reverse
+from lxml.etree import XPath
 from lxml.html import fromstring
 from maastesting.matchers import ContainsAll
 from maasserver.testing import (
@@ -22,6 +23,7 @@
 from maasserver.testing.testcase import (
     LoggedInTestCase,
     )
+from maasserver.views import tags as tags_views
 
 
 class TagViewsTest(LoggedInTestCase):
@@ -90,3 +92,47 @@
         content_text = doc.cssselect('#content')[0].text_content()
         self.assertIn(node.hostname, content_text)
         self.assertNotIn(node2.hostname, content_text)
+
+    def test_view_tag_paginates_nodes(self):
+        """Listing of nodes with tag is split across multiple pages
+
+        Copy-coded from NodeViewsTest.test_node_list_paginates evilly.
+        """
+        # Set a very small page size to save creating lots of nodes
+        page_size = 2
+        self.patch(tags_views.TagView, 'paginate_by', page_size)
+        tag = factory.make_tag()
+        nodes = [factory.make_node(created="2012-10-12 12:00:%02d" % i)
+            for i in range(page_size * 2 + 1)]
+        for node in nodes:
+            node.tags = [tag]
+        # Order node links with newest first as the view is expected to
+        node_links = [reverse('node-view', args=[node.system_id])
+            for node in reversed(nodes)]
+        expr_node_links = XPath("//div[@id='nodes']/table//a/@href")
+        expr_page_anchors = XPath("//div[@class='pagination']//a")
+        # Fetch first page, should link newest two nodes and page 2
+        response = self.client.get(reverse('tag-view', args=[tag.name]))
+        page1 = fromstring(response.content)
+        self.assertEqual(node_links[:page_size], expr_node_links(page1))
+        self.assertEqual([("next", "?page=2"), ("last", "?page=3")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page1)])
+        # Fetch second page, should link next nodes and adjacent pages
+        response = self.client.get(reverse('tag-view', args=[tag.name]),
+            {"page": 2})
+        page2 = fromstring(response.content)
+        self.assertEqual(node_links[page_size:page_size * 2],
+            expr_node_links(page2))
+        self.assertEqual([("first", "."), ("previous", "."),
+                ("next", "?page=3"), ("last", "?page=3")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page2)])
+        # Fetch third page, should link oldest node and node list page
+        response = self.client.get(reverse('tag-view', args=[tag.name]),
+            {"page": 3})
+        page3 = fromstring(response.content)
+        self.assertEqual(node_links[page_size * 2:], expr_node_links(page3))
+        self.assertEqual([("first", "."), ("previous", "?page=2")],
+            [(a.text.lower(), a.get("href"))
+                for a in expr_page_anchors(page3)])

=== modified file 'src/maasserver/views/tags.py'
--- src/maasserver/views/tags.py	2012-10-10 09:43:17 +0000
+++ src/maasserver/views/tags.py	2012-11-07 11:44:25 +0000
@@ -15,28 +15,29 @@
     ]
 
 from maasserver.models import Tag
-from django.views.generic import (
-    UpdateView,
-    )
-
-
-class TagView(UpdateView):
+from maasserver.views import PaginatedListView
+
+
+class TagView(PaginatedListView):
     """Basic view of a tag.
     """
 
     template_name = 'maasserver/tag_view.html'
-    context_object_name = 'tag'
+    context_object_name = 'node_list'
 
-    def get_object(self):
-        name = self.kwargs.get('name', None)
-        tag = Tag.objects.get_tag_or_404(
-            name=name, user=self.request.user,
+    def get(self, request, *args, **kwargs):
+        self.tag = Tag.objects.get_tag_or_404(
+            name=kwargs.get('name', None),
+            user=self.request.user,
             to_edit=False)
-        return tag
+        return super(TagView, self).get(request, *args, **kwargs)
+
+    def get_queryset(self):
+        return Tag.objects.get_nodes(
+            self.tag, user=self.request.user, prefetch_mac=True,
+            ).order_by('-created')
 
     def get_context_data(self, **kwargs):
         context = super(TagView, self).get_context_data(**kwargs)
-        nodes = Tag.objects.get_nodes(context['tag'], self.request.user,
-            prefetch_mac=True)
-        context['node_list'] = nodes
+        context['tag'] = self.tag
         return context