mirror
/
wagtail
kopia lustrzana https://github.com/wagtail/wagtail
1
0
Forkuj 0
Kod Zgłoszenia Packages Projekty Wydania Wiki Aktywność

Revert "Implement `page_url_path_changed` signal (RFC 34) (1 of 3)" (#7797) 

* Revert "Release notes for #7761"

This reverts commit b062c22331.

* Revert "Fix incorrect logging of move VS reorder"

This reverts commit 0086c7ba7a.

* Revert "Update docs"

This reverts commit e602990e39.

* Revert "Add tests"

This reverts commit b10e545b14.

* Revert "Prevent the 'old_record' query being unnecessary triggered by page.move()"

This reverts commit 4fed675d7a.

* Revert "Implement the page_url_path_changed signal"

This reverts commit 2f86eda372.
pull/7799/head
Karl Hobley 2021-12-18 16:54:44 +00:00 zatwierdzone przez GitHub
rodzic 6f2c576da0
commit 247030df14
Nie znaleziono w bazie danych klucza dla tego podpisu
ID klucza GPG: 4AEE18F83AFDEB23
7 zmienionych plików z 15 dodań i 191 usunięć
Pokaż statystyki Ściągnij plik aktualizacji Ściągnij plik porównania Expand all files Collapse all files

1
CHANGELOG.txt
Wyświetl plik

@ -13,7 +13,6 @@ Changelog
* Add support for Azure CDN and Front Door front-end cache invalidation (Tomasz Knapik)
* Fix: Accessibility fixes for Windows high contrast mode; Dashboard icons colour and contrast (Sakshi Uppoor)
* Fix: Rename additional 'spin' CSS animations to avoid clashes with other libraries (Kevin Gutiérrez)
* Add `page_url_path_changed` signal for pages (Andy Babic)
2.15.2 (xx.xx.xxxx) - IN DEVELOPMENT

18
docs/reference/signals.rst
Wyświetl plik

@ -127,24 +127,6 @@ The best way to distinguish between a 'move' and 'reorder' is to compare the ``u
# Register a receiver
pre_page_move.connect(clear_old_page_urls_from_cache)
``page_url_path_changed``
-------------------------
This signal is emitted from a ``Page`` after its ``url_path`` value has changed as a result of:
- The publishing of a change to it's `slug`.
- A page being moved to a different part of the tree.
If you rely on Wagtail's built-in methods for URL generation and routing, a change to the ``url_path`` value
is essentially the best indicator that the URL of a page (and most likely all of its descendants) has changed.
The following arguments are emitted by this signal:
:sender: The page ``class``.
:instance: The generic ``Page`` instance.
:url_path_before: The value of ``instance.url_path`` **before** the change.
:url_path_after: The value of ``instance.url_path`` **after** the change.
:log_entry: A ``PageLogEntry`` instance detailing the action that triggered the signal to be emitted. This value could be `None` if the action was unclear.
workflow_submitted
------------------

1
docs/releases/2.16.md
Wyświetl plik

@ -17,7 +17,6 @@
* Private pages can now be fetched over the API (Nabil Khalil)
* Added `alias_of` field to the pages API (Dmitrii Faiazov)
* Add support for Azure CDN and Front Door front-end cache invalidation (Tomasz Knapik)
* Add `page_url_path_changed` signal for Pages (Andy Babic)
### Bug fixes

55
wagtail/core/models/__init__.py
Wyświetl plik

@ -52,9 +52,9 @@ from wagtail.core.forms import TaskStateCommentForm
from wagtail.core.log_actions import log
from wagtail.core.query import PageQuerySet
from wagtail.core.signals import (
page_published, page_unpublished, page_url_path_changed, post_page_move, pre_page_move,
pre_validate_delete, task_approved, task_cancelled, task_rejected, task_submitted,
workflow_approved, workflow_cancelled, workflow_rejected, workflow_submitted)
page_published, page_unpublished, post_page_move, pre_page_move, pre_validate_delete,
task_approved, task_cancelled, task_rejected, task_submitted, workflow_approved,
workflow_cancelled, workflow_rejected, workflow_submitted)
from wagtail.core.treebeard import TreebeardPathFixMixin
from wagtail.core.url_routing import RouteResult
from wagtail.core.utils import (
@ -482,8 +482,8 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
if clean:
self.full_clean()
update_descendant_url_paths = False
is_new = self.id is None
slug_changed = False
if is_new:
# we are creating a record. If we're doing things properly, this should happen
@ -499,18 +499,18 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
old_record = Page.objects.get(id=self.id)
if old_record.slug != self.slug:
self.set_url_path(self.get_parent())
slug_changed = True
update_descendant_url_paths = True
old_url_path = old_record.url_path
new_url_path = self.url_path
result = super().save(**kwargs)
if slug_changed:
if not is_new and update_descendant_url_paths:
self._update_descendant_url_paths(old_url_path, new_url_path)
# Check if this is a root page of any sites and clear the 'wagtail_site_root_paths' key if so
# Note: New translations of existing site roots are considered site roots as well, so we must
# always check if this page is a site root, even if it's new.
# always check if this page is a site root, even if it's new.
if self.is_site_root():
cache.delete('wagtail_site_root_paths')
@ -526,35 +526,24 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
self.url_path
)
log_entry = None
if log_action is not None:
# The default for log_action is False. i.e. don't log unless specifically instructed
# Page creation is a special case that we want logged by default, but allow skipping it
# explicitly by passing log_action=None
if is_new:
log_entry = log(
log(
instance=self,
action='wagtail.create',
user=user or self.owner,
content_changed=True,
)
elif log_action:
log_entry = log(
log(
instance=self,
action=log_action,
user=user
)
if slug_changed:
# Emit page_url_path_changed signal only on successful commit
transaction.on_commit(lambda: page_url_path_changed.send(
sender=self.specific_class or self.__class__,
instance=self,
url_path_before=old_url_path,
url_path_after=new_url_path,
log_entry=log_entry,
))
return result
def delete(self, *args, **kwargs):
@ -1503,7 +1492,6 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
old_self = Page.objects.get(id=self.id)
old_url_path = old_self.url_path
new_url_path = old_self.set_url_path(parent=parent_after)
url_path_changed = old_url_path != new_url_path
# Emit pre_page_move signal
pre_page_move.send(
@ -1523,13 +1511,11 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
# Treebeard's move method doesn't actually update the in-memory instance,
# so we need to work with a freshly loaded one now
new_self = Page.objects.get(id=self.id)
# Update url_path without triggering slug-change handling in save()
new_self.url_path = new_url_path
new_self.save(update_fields=["url_path"])
new_self.save()
# Update descendant paths if url_path has changed
if url_path_changed:
if old_url_path != new_url_path:
new_self._update_descendant_url_paths(old_url_path, new_url_path)
# Emit post_page_move signal
@ -1543,10 +1529,10 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
)
# Log
log_entry = log(
log(
instance=self,
# Check if page was reordered (reordering doesn't change the parent)
action='wagtail.move' if parent_before.id != parent_after.id else 'wagtail.reorder',
action='wagtail.reorder' if parent_before.id == target.id else 'wagtail.move',
user=user,
data={
'source': {
@ -1561,16 +1547,6 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
)
logger.info("Page moved: \"%s\" id=%d path=%s", self.title, self.id, new_url_path)
if url_path_changed:
# Emit page_url_path_changed signal
page_url_path_changed.send(
sender=self.specific_class or self.__class__,
instance=self,
url_path_before=old_url_path,
url_path_after=new_url_path,
log_entry=log_entry,
)
def copy(self, recursive=False, to=None, update_attrs=None, copy_revisions=True, keep_live=True, user=None,
process_child_object=None, exclude_fields=None, log_action='wagtail.copy', reset_translation_key=True):
"""
@ -1997,10 +1973,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase):
def get_cached_paths(self):
"""
This returns a list of paths to invalidate in a frontend cache, or when
create redirects when the page's URL changes as a result of a slug
change or move. These values are automatically added to the page's
default URL to create full URLs.
This returns a list of paths to invalidate in a frontend cache
"""
return ['/']

3
wagtail/core/signals.py
Wyświetl plik

@ -9,9 +9,6 @@ page_published = Signal()
# provides args: instance
page_unpublished = Signal()
# provides args: instance, url_path_before, url_path_after, log_entry
page_url_path_changed = Signal()
# provides args: instance, parent_page_before, parent_page_after, url_path_before, url_path_after
pre_page_move = Signal()

5
wagtail/core/tests/test_audit_log.py
Wyświetl plik

@ -209,10 +209,7 @@ class TestAuditLog(TestCase):
instance=SimplePage(title="About us", slug="about", content="hello")
)
user = get_user_model().objects.first()
# move() interprets `target` as an intended 'sibling' by default, so
# we must use `pos` to indicate that `self.home_page` should be the
# new 'parent'
section.move(self.home_page, pos="last-child", user=user)
section.move(self.home_page, user=user)
self.assertEqual(PageLogEntry.objects.filter(action='wagtail.move', user=user).count(), 1)
self.assertEqual(PageLogEntry.objects.filter(action='wagtail.reorder', user=user).count(), 0)

123
wagtail/core/tests/test_signals.py
Wyświetl plik

@ -1,123 +0,0 @@
from unittest import mock
from django.db import connection
from django.test import TestCase
from wagtail.core.models import Site
from wagtail.core.signals import page_url_path_changed
from wagtail.tests.testapp.models import SimplePage
from wagtail.tests.utils import WagtailTestUtils
class TestPageURLPathChangedSignal(TestCase, WagtailTestUtils):
"""
Tests for the `wagtail.core.signals.page_url_path_changed` signal
"""
def setUp(self):
# Find root page
site = Site.objects.select_related('root_page').get(is_default_site=True)
root_page = site.root_page
# Create two sections
self.section_a = SimplePage(title="Section A", slug="section-a", content="hello")
root_page.add_child(instance=self.section_a)
self.section_b = SimplePage(title="Section B", slug="section-b", content="hello")
root_page.add_child(instance=self.section_b)
# Add test pages to section A
self.test_page = SimplePage(title="Hello world! A", slug="hello-world-a", content="hello")
self.section_a.add_child(instance=self.test_page)
def test_signal_emitted_on_slug_change(self):
# Connect a mock signal handler to the signal
handler = mock.MagicMock()
page_url_path_changed.connect(handler)
try:
self.test_page.slug = 'updated'
self.test_page.save()
# TODO: When Django 3.1< support is dropped, wrap save in
# self.captureOnCommitCallbacks and remove this code
for _, func in connection.run_on_commit:
func()
finally:
# Disconnect mock handler to prevent cross-test pollution
page_url_path_changed.disconnect(handler)
# Check the signal was fired
self.assertEqual(handler.call_count, 1)
self.assertTrue(
handler.called_with(
sender=SimplePage,
instance=self.test_page,
url_path_before='/home/section-a/hello-world-a/',
url_path_after='/home/section-a/updated/',
log_entry=mock.ANY,
)
)
def test_signal_emitted_on_page_move(self):
# Connect a mock signal handler to the signal
handler = mock.MagicMock()
page_url_path_changed.connect(handler)
try:
self.test_page.move(self.section_b, pos="last-child")
finally:
# Disconnect mock handler to prevent cross-test pollution
page_url_path_changed.disconnect(handler)
# Check the signal was fired
self.assertEqual(handler.call_count, 1)
self.assertTrue(
handler.called_with(
sender=SimplePage,
instance=self.test_page,
url_path_before='/home/section-a/hello-world-a/',
url_path_after='/home/section-b/hello-world-a/',
log_entry=mock.ANY,
)
)
def test_signal_not_emitted_on_title_change(self):
# Connect a mock signal handler to the signal
handler = mock.MagicMock()
page_url_path_changed.connect(handler)
try:
self.test_page.title = 'Goodnight Moon!'
self.test_page.save()
# NOTE: Even though we're not expecting anything to happen here,
# we need to invoke the callbacks in run_on_commit the same way
# the same way we do in ``test_signal_emitted_on_slug_change``,
# otherwise this test wouldn't prove anything.
for _, func in connection.run_on_commit:
func()
finally:
# Disconnect mock handler to prevent cross-test pollution
page_url_path_changed.disconnect(handler)
# Check the signal was NOT fired
self.assertEqual(handler.call_count, 0)
def test_signal_not_emitted_when_reordering(self):
# Add a couple of siblings for the test page to make reording more apparent
self.section_a.add_child(instance=SimplePage(title="Sibling 1", slug="sibling-one", content="foo"))
self.section_a.add_child(instance=SimplePage(title="Sibling 2", slug="sibling-two", content="foo"))
# Connect a mock signal handler to the signal
handler = mock.MagicMock()
page_url_path_changed.connect(handler)
try:
# Move 'test_page' from first to last place (under the same parent)
self.test_page.move(self.test_page.get_parent(), pos="last-child")
self.test_page.save()
finally:
# Disconnect mock handler to prevent cross-test pollution
page_url_path_changed.disconnect(handler)
# Check the signal was NOT fired
self.assertEqual(handler.call_count, 0)