Browse Source

Adopt the pageurl template tag over `page.url` where possible

- Fixes #10125
- So that determining page URL is more performant - see https://docs.wagtail.org/en/stable/advanced_topics/performance.html#page-urls
Satvik Vashisht 2 years ago
parent
commit
4ee60b5d47

+ 1 - 0
CHANGELOG.txt

@@ -74,6 +74,7 @@ Changelog
  * Maintenance: Update Algolia DocSearch to use new application and correct versioning setup (Thibaud Colas)
  * Maintenance: Move snippet choosers and model check registration to `SnippetViewSet.on_register()` (Sage Abdullah)
  * Maintenance: Remove unused snippets delete-multiple view (Sage Abdullah)
+ * Maintenance: Improve performance of determining live page URLs across the admin interface using `pageurl` template tag (Satvik Vashisht)
 
 
 4.2.1 (13.03.2023)

+ 2 - 0
docs/advanced_topics/performance.md

@@ -58,6 +58,8 @@ Another side benefit is it prevents errors during conversation from causing page
 
 The same can be achieved in Python using [`generate_image_url`](dynamic_image_urls).
 
+(performance_page_urls)=
+
 ## Page URLs
 
 To fully resolve the URL of a page, Wagtail requires information from a few different sources.

+ 1 - 0
docs/releases/5.0.md

@@ -95,6 +95,7 @@ Support for adding custom validation logic to StreamField blocks has been formal
  * Update Algolia DocSearch to use new application and correct versioning setup (Thibaud Colas)
  * Move snippet choosers and model check registration to `SnippetViewSet.on_register()` (Sage Abdullah)
  * Remove unused snippets delete-multiple view (Sage Abdullah)
+ * Improve performance of determining live page URLs across the admin interface using [`pageurl` template tag](performance_page_urls) (Satvik Vashisht)
 
 
 ## Upgrade considerations

+ 2 - 1
wagtail/admin/templates/wagtailadmin/chooser/tables/page_title_cell.html

@@ -1,11 +1,12 @@
 {% load l10n wagtailadmin_tags %}
+{% load wagtailcore_tags %}
 <td class="{% if column.classname %}{{ column.classname }} {% endif %}title">
     <div class="title-wrapper">
         {% if page.can_choose %}
             {% if column.is_multiple_choice %}
                 <label for="chooser-modal-select-{{ page.id|unlocalize }}">{{ value }}</label>
             {% else %}
-                <a class="choose-page" href="#{{ page.id|unlocalize }}" data-id="{{ page.id|unlocalize }}" data-title="{{ page.title }}" data-admin-title="{{ page.get_admin_display_title }}" data-url="{{ page.url }}" data-parent-id="{{ page.get_parent.id|unlocalize }}" data-edit-url="{% url 'wagtailadmin_pages:edit' page.id %}">{{ value }}</a>
+                <a class="choose-page" href="#{{ page.id|unlocalize }}" data-id="{{ page.id|unlocalize }}" data-title="{{ page.title }}" data-admin-title="{{ page.get_admin_display_title }}" data-url="{% pageurl page %}" data-parent-id="{{ page.get_parent.id|unlocalize }}" data-edit-url="{% url 'wagtailadmin_pages:edit' page.id %}">{{ value }}</a>
             {% endif %}
         {% else %}
             {{ value }}

+ 5 - 5
wagtail/admin/templates/wagtailadmin/home/locked_pages.html

@@ -1,4 +1,5 @@
 {% load i18n wagtailadmin_tags %}
+{% load wagtailcore_tags %}
 {% if locked_pages %}
     {% trans "Your locked pages" as heading %}
     {% panel id="locked-pages" heading=heading %}
@@ -48,11 +49,10 @@
                                     <li><a href="{% url 'wagtailadmin_pages:view_draft' page.id %}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Draft' %}</a></li>
                                 {% endif %}
                                 {% if page.live %}
-                                    {% with page_url=page.url %}
-                                        {% if page_url is not None %}
-                                            <li><a href="{{ page_url }}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Live' %}</a></li>
-                                        {% endif %}
-                                    {% endwith %}
+                                    {% pageurl page as page_url %}
+                                    {% if page_url is not None %}
+                                        <li><a href="{{ page_url }}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Live' %}</a></li>
+                                    {% endif %}
                                 {% endif %}
                             </ul>
                         </td>

+ 5 - 6
wagtail/admin/templates/wagtailadmin/home/recent_edits.html

@@ -1,5 +1,5 @@
 
-
+{% load wagtailcore_tags %}
 {% load i18n wagtailadmin_tags %}
 {% if last_edits %}
     {% trans "Your most recent edits" as heading %}
@@ -36,11 +36,10 @@
                                     <li><a href="{% url 'wagtailadmin_pages:view_draft' page.id %}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Draft' %}</a></li>
                                 {% endif %}
                                 {% if page.live %}
-                                    {% with page_url=page.url %}
-                                        {% if page_url is not None %}
-                                            <li><a href="{{ page_url }}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Live' %}</a></li>
-                                        {% endif %}
-                                    {% endwith %}
+                                    {% pageurl page as page_url %}
+                                    {% if page_url is not None %}
+                                        <li><a href="{{ page_url }}" class="button button-small button-secondary" target="_blank" rel="noreferrer">{% trans 'Live' %}</a></li>
+                                    {% endif %}
                                 {% endif %}
                             </ul>
                         </td>

+ 8 - 9
wagtail/admin/templates/wagtailadmin/shared/page_status_tag.html

@@ -1,14 +1,13 @@
+{% load wagtailcore_tags %}
 {% load i18n wagtailadmin_tags %}
+{% trans "Current page status:" as status_hidden_label %}
 {% if page.live %}
-    {% with page_url=page.url %}
-        {% trans "Current page status:" as status_hidden_label %}
-        {% if page_url is not None %}
-            {% trans 'Visit the live page' as status_title %}
-            {% status page.status_string url=page_url title=status_title hidden_label=status_hidden_label classname="primary" attrs='target="_blank" rel="noreferrer"' %}
-        {% else %}
-            {% status page.status_string hidden_label=status_hidden_label classname="primary" %}
-        {% endif %}
-    {% endwith %}
+    {% pageurl page as page_url %}
+    {% if page_url is not None %}
+        {% status page.status_string url=page_url title=_("Visit the live page") hidden_label=status_hidden_label classname="primary" attrs='target="_blank" rel="noreferrer"' %}
+    {% else %}
+        {% status page.status_string hidden_label=status_hidden_label classname="primary" %}
+    {% endif %}
 {% else %}
     {% status page.status_string hidden_label=status_hidden_label %}
 {% endif %}

+ 1 - 0
wagtail/admin/templates/wagtailadmin/shared/page_status_tag_new.html

@@ -7,6 +7,7 @@
     - `classes` - String of extra css classes to pass to this component
 {% endcomment %}
 
+{% comment %} Unable to use pageurl template tag here due to issues in unit tests where request is not yet available - see #10157 {% endcomment %}
 {% if page.live and page.url is not None %}
     {% test_page_is_public page as is_public %}
 

+ 4 - 2
wagtail/admin/tests/pages/test_dashboard.py

@@ -3,6 +3,7 @@ from django.test import TestCase
 from django.urls import reverse
 
 from wagtail.admin.views.home import RecentEditsPanel
+from wagtail.coreutils import get_dummy_request
 from wagtail.models import Page
 from wagtail.test.testapp.models import SimplePage
 from wagtail.test.utils import WagtailTestUtils
@@ -121,7 +122,8 @@ class TestRecentEditsQueryCount(WagtailTestUtils, TestCase):
 
     def setUp(self):
         self.bob = self.create_superuser(username="bob", password="password")
-
+        self.dummy_request = get_dummy_request()
+        self.dummy_request.user = self.bob
         # make a bunch of page edits (all to EventPages, so that calls to specific() don't add
         # an unpredictable number of queries)
         pages_to_edit = Page.objects.filter(id__in=[4, 5, 6, 9, 12, 13]).specific()
@@ -135,7 +137,7 @@ class TestRecentEditsQueryCount(WagtailTestUtils, TestCase):
             # Instantiating/getting context of RecentEditsPanel should not generate N+1 queries -
             # i.e. any number less than 6 would be reasonable here
             panel = RecentEditsPanel()
-            parent_context = {"request": self.client}
+            parent_context = {"request": self.dummy_request}
             panel.get_context_data(parent_context)
 
         # check that the panel is still actually returning results