From 5c9ff23e229acabad406c42c4e13cbaea32e6c15 Mon Sep 17 00:00:00 2001 From: Andy Chosak Date: Thu, 18 Jan 2018 14:26:27 -0500 Subject: [PATCH] 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 | 1 + docs/releases/2.1.rst | 1 + wagtail/admin/site_summary.py | 34 +++++--- .../wagtailadmin/home/site_summary_pages.html | 2 +- wagtail/admin/tests/test_site_summary.py | 83 +++++++++++++++++++ wagtail/admin/tests/tests.py | 37 +-------- 6 files changed, 109 insertions(+), 49 deletions(-) create mode 100644 wagtail/admin/tests/test_site_summary.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index d5ade9a8bd..cb22de38bf 100644 --- a/CHANGELOG.txt +++ b/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) diff --git a/docs/releases/2.1.rst b/docs/releases/2.1.rst index c0cf150fbf..3174e871c6 100644 --- a/docs/releases/2.1.rst +++ b/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 diff --git a/wagtail/admin/site_summary.py b/wagtail/admin/site_summary.py index 3bd7dcea06..cdb339ee3e 100644 --- a/wagtail/admin/site_summary.py +++ b/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): diff --git a/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html b/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html index 51d552d68a..0fa518617e 100644 --- a/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html +++ b/wagtail/admin/templates/wagtailadmin/home/site_summary_pages.html @@ -1,7 +1,7 @@ {% load i18n wagtailadmin_tags %}
  • - + {% blocktrans count counter=total_pages with total_pages|intcomma as total %} {{ total }} Page {% plural %} diff --git a/wagtail/admin/tests/test_site_summary.py b/wagtail/admin/tests/test_site_summary.py new file mode 100644 index 0000000000..71a11e91e4 --- /dev/null +++ b/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("{} 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("0 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("1 Page") diff --git a/wagtail/admin/tests/tests.py b/wagtail/admin/tests/tests.py index 07e719302f..b5d7512e47 100644 --- a/wagtail/admin/tests/tests.py +++ b/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 = ''.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 = ''.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 = ''.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)