make site summary panel respect user privileges

This change modifies how the Wagtail home site summary panel displays
the number of pages on the site, and where that number links to.

Instead of showing the total number of pages on the site, the panel
should show the number of pages under the user's explorable root page
(inclusive). If the user has access to the full tree, the Wagtail root
is not counted in this total.

Previously, the site summary page link would go to the Wagtail root if
there were multiple sites in an installation, and to the site root page
for a single site. This change modifies this logic so that the link
always goes to the user's explorable root page (which may be their
explorable root page).

The unit tests for the site summary panel have been pulled out into a
new module at `wagtail.admin.tests.test_site_summary`, and augmented to
test how things work for users with different permissions.

CHANGELOG.txt

@@ -47,6 +47,7 @@ Changelog
  * Fix: 'Add user' is now rendered as a button due to the use of quotes within translations (Benoît Vogel)
  * Fix: Menu icon no longer overlaps with title in Modeladmin on mobile (Coen van der Kamp)
  * Fix: Background color overflow within the Wagtail documentation (Sergey Fedoseev)
+ * Fix: Page count on homepage summary panel now takes account of user permissions (Andy Chosak)
 
 
 2.0.1 (04.04.2018)

docs/releases/2.1.rst

@@ -70,6 +70,7 @@ Bug fixes
  * 'Add user' is now rendered as a button due to the use of quotes within translations (Benoît Vogel)
  * Menu icon no longer overlaps with title in Modeladmin on mobile (Coen van der Kamp)
  * Background color overflow within the Wagtail documentation (Sergey Fedoseev)
+ * Page count on homepage summary panel now takes account of user permissions (Andy Chosak)
 
 
 Upgrade considerations

wagtail/admin/site_summary.py

@@ -1,5 +1,6 @@
 from django.template.loader import render_to_string
 
+from wagtail.admin.navigation import get_explorable_root_page
 from wagtail.admin.utils import user_has_any_page_permission
 from wagtail.core import hooks
 from wagtail.core.models import Page, Site
@@ -26,20 +27,29 @@ class PagesSummaryItem(SummaryItem):
     template = 'wagtailadmin/home/site_summary_pages.html'
 
     def get_context(self):
-        # If there is a single site, link to the homepage of that site
-        # Otherwise, if there are multiple sites, link to the root page
-        try:
-            site = Site.objects.get()
-            root = site.root_page
-            single_site = True
-        except (Site.DoesNotExist, Site.MultipleObjectsReturned):
-            root = None
-            single_site = False
+        root_page = get_explorable_root_page(self.request.user)
+
+        if root_page:
+            page_count = Page.objects.descendant_of(root_page, inclusive=True).count()
+
+            if root_page.is_root():
+                # If the root page the user has access to is the Wagtail root,
+                # subtract one from this count because the root is not a real page.
+                page_count -= 1
+
+                # If precisely one site exists, link to its homepage rather than the
+                # tree root, to discourage people from trying to create pages as siblings
+                # of the homepage (#1883)
+                try:
+                    root_page = Site.objects.get().root_page
+                except (Site.DoesNotExist, Site.MultipleObjectsReturned):
+                    pass
+        else:
+            page_count = 0
 
         return {
-            'single_site': single_site,
-            'root_page': root,
-            'total_pages': Page.objects.count() - 1,  # subtract 1 because the root node is not a real page
+            'root_page': root_page,
+            'total_pages': page_count,
         }
 
     def is_shown(self):

wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html

@@ -1,7 +1,7 @@
 {% load i18n wagtailadmin_tags %}
 
 <li class="icon icon-doc-empty-inverse">
-<a href="{% if single_site %}{% url 'wagtailadmin_explore' root_page.pk %}{% else %}{% url 'wagtailadmin_explore_root' %}{% endif %}">
+<a href="{% url 'wagtailadmin_explore' root_page.pk %}">
     {% blocktrans count counter=total_pages with total_pages|intcomma as total %}
         <span>{{ total }}</span> Page
     {% plural %}

wagtail/admin/tests/test_site_summary.py

@@ -0,0 +1,83 @@
+from django.contrib.auth.models import Group
+from django.test import TestCase
+from django.urls import reverse
+
+from wagtail.admin.site_summary import PagesSummaryItem
+from wagtail.core.models import GroupPagePermission, Page, Site
+from wagtail.tests.testapp.models import SimplePage
+from wagtail.tests.utils import WagtailTestUtils
+
+
+class TestPagesSummary(TestCase, WagtailTestUtils):
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+
+        cls.test_page = SimplePage(title="test", slug='test', content="test")
+        cls.wagtail_root = Page.get_first_root_node()
+        cls.wagtail_root.add_child(instance=cls.test_page)
+
+        cls.test_page_group = Group.objects.create(name="Test page")
+        GroupPagePermission.objects.create(
+            group=cls.test_page_group,
+            page=cls.test_page,
+            permission_type='edit'
+        )
+
+    @classmethod
+    def tearDownClass(cls):
+        cls.test_page.delete()
+        cls.test_page_group.delete()
+
+        super().tearDownClass()
+
+    def setUp(self):
+        self.user = self.login()
+        self.request = self.client.get('/').wsgi_request
+
+    def assertSummaryContains(self, content):
+        summary = PagesSummaryItem(self.request).render()
+        self.assertIn(content, summary)
+
+    def assertSummaryContainsLinkToPage(self, page_pk):
+        self.assertSummaryContains(reverse('wagtailadmin_explore', args=[page_pk]))
+
+    def test_user_with_page_permissions_is_shown_panel(self):
+        self.assertTrue(PagesSummaryItem(self.request).is_shown())
+
+    def test_single_site_summary_links_to_site_root(self):
+        self.assertEqual(Site.objects.count(), 1)
+        site = Site.objects.first()
+        self.assertSummaryContainsLinkToPage(site.root_page.pk)
+
+    def test_multiple_sites_summary_links_to_wagtail_root(self):
+        Site.objects.create(hostname='foo.com', root_page=self.wagtail_root)
+        self.assertSummaryContainsLinkToPage(self.wagtail_root.pk)
+
+    def test_no_sites_summary_links_to_wagtail_root(self):
+        Site.objects.all().delete()
+        self.assertSummaryContainsLinkToPage(self.wagtail_root.pk)
+
+    def test_summary_includes_page_count_without_wagtail_root(self):
+        self.assertSummaryContains("<span>{}</span> Pages".format(Page.objects.count() - 1))
+
+    def test_summary_shows_zero_pages_if_none_exist_except_wagtail_root(self):
+        Page.objects.exclude(pk=self.wagtail_root.pk).delete()
+        self.assertSummaryContains("<span>0</span> Pages")
+
+    def test_user_with_no_page_permissions_is_not_shown_panel(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.assertFalse(PagesSummaryItem(self.request).is_shown())
+
+    def test_user_with_limited_page_permissions_summary_links_to_their_root(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.user.groups.add(self.test_page_group)
+        self.assertSummaryContainsLinkToPage(self.test_page.pk)
+
+    def test_user_with_limited_page_permissions_sees_proper_page_count(self):
+        self.user.is_superuser = False
+        self.user.save()
+        self.user.groups.add(self.test_page_group)
+        self.assertSummaryContains("<span>1</span> Page")

wagtail/admin/tests/tests.py

@@ -11,9 +11,8 @@ from django.utils.translation import ugettext_lazy as _
 from taggit.models import Tag
 
 from wagtail.admin.menu import MenuItem
-from wagtail.admin.site_summary import PagesSummaryItem
 from wagtail.admin.utils import send_mail, user_has_any_page_permission
-from wagtail.core.models import Page, Site
+from wagtail.core.models import Page
 from wagtail.tests.utils import WagtailTestUtils
 
 
@@ -73,40 +72,6 @@ class TestHome(TestCase, WagtailTestUtils):
         self.assertEqual(response.status_code, 200)
 
 
-class TestPagesSummary(TestCase, WagtailTestUtils):
-    def setUp(self):
-        self.login()
-
-    def get_request(self):
-        """
-        Get a Django WSGI request that has been passed through middleware etc.
-        """
-        return self.client.get('/admin/').wsgi_request
-
-    def test_page_summary_single_site(self):
-        request = self.get_request()
-        root_page = request.site.root_page
-        link = '<a href="{}">'.format(reverse('wagtailadmin_explore', args=[root_page.pk]))
-        page_summary = PagesSummaryItem(request)
-        self.assertIn(link, page_summary.render())
-
-    def test_page_summary_multiple_sites(self):
-        Site.objects.create(
-            hostname='example.com',
-            root_page=Page.objects.get(pk=1))
-        request = self.get_request()
-        link = '<a href="{}">'.format(reverse('wagtailadmin_explore_root'))
-        page_summary = PagesSummaryItem(request)
-        self.assertIn(link, page_summary.render())
-
-    def test_page_summary_zero_sites(self):
-        Site.objects.all().delete()
-        request = self.get_request()
-        link = '<a href="{}">'.format(reverse('wagtailadmin_explore_root'))
-        page_summary = PagesSummaryItem(request)
-        self.assertIn(link, page_summary.render())
-
-
 class TestEditorHooks(TestCase, WagtailTestUtils):
     def setUp(self):
         self.homepage = Page.objects.get(id=2)