diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 769098b357..ea40771924 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -56,6 +56,7 @@ Changelog * Fix: Ensure Page querysets support using `alias` and `specific` (Tomasz Knapik) * Fix: Ensure workflow dashboard panels work when the page/snippet is missing (Sage Abdullah) * Fix: Prevent a ValueError with `FormSubmissionsPanel` on Django 5.0 when creating a new form page (Matt Westcott) + * Fix: Avoid duplicate entries in "Recent edits" panel when copying pages (Matt Westcott) * Docs: Document, for contributors, the use of translate string literals passed as arguments to tags and filters using `_()` within templates (Chiemezuo Akujobi) * Docs: Document all features for the Documents app in one location (Neeraj Yetheendran) * Docs: Add section to testing docs about creating pages and working with page content (Mariana Bedran Lesche) diff --git a/docs/releases/6.0.md b/docs/releases/6.0.md index ee05667ca9..e7da23010a 100644 --- a/docs/releases/6.0.md +++ b/docs/releases/6.0.md @@ -84,6 +84,7 @@ Thank you to Thibaud Colas and Badr Fourane for their work on this feature. * Ensure `ActionController` explicitly checks for elements that allow select functionality (Nandini Arora) * Prevent a ValueError with `FormSubmissionsPanel` on Django 5.0 when creating a new form page (Matt Westcott) * Add ability to [customise a page's copy form](custom_page_copy_form) including an auto-incrementing slug example (Neeraj Yetheendran) + * Avoid duplicate entries in "Recent edits" panel when copying pages (Matt Westcott) ### Documentation diff --git a/wagtail/admin/templates/wagtailadmin/home/recent_edits.html b/wagtail/admin/templates/wagtailadmin/home/recent_edits.html index 7b53aa646f..577067a203 100644 --- a/wagtail/admin/templates/wagtailadmin/home/recent_edits.html +++ b/wagtail/admin/templates/wagtailadmin/home/recent_edits.html @@ -15,7 +15,7 @@ - {% for revision, page in last_edits %} + {% for last_edited_at, page in last_edits %}
@@ -49,7 +49,7 @@ {% include "wagtailadmin/shared/page_status_tag.html" with page=page %} - {% human_readable_date revision.created_at %} + {% human_readable_date last_edited_at %} {% endfor %} diff --git a/wagtail/admin/tests/pages/test_dashboard.py b/wagtail/admin/tests/pages/test_dashboard.py index e420338719..a1f92277b1 100644 --- a/wagtail/admin/tests/pages/test_dashboard.py +++ b/wagtail/admin/tests/pages/test_dashboard.py @@ -1,6 +1,8 @@ from django.contrib.auth import get_user_model from django.test import TestCase from django.urls import reverse +from django.utils import timezone +from freezegun import freeze_time from wagtail.admin.views.home import RecentEditsPanel from wagtail.coreutils import get_dummy_request @@ -84,17 +86,13 @@ class TestRecentEditsPanel(WagtailTestUtils, TestCase): self.assertIn("Your most recent edits", response.content.decode("utf-8")) def test_missing_page_record(self): - # Ensure that the panel still renders when one of the returned revision records - # has no corresponding Page object. It's unclear how this happens, since revisions - # are deleted on page deletion, but there are reports of this happening in - # https://github.com/wagtail/wagtail/issues/9185 - - # edit the revision object to be owned by Alice and have an unrecognised object ID - self.revision.user = self.user_alice - self.revision.object_id = "999999" - self.revision.save() + # Ensure that the panel still renders when one of the page IDs returned from querying + # PageLogEntry has no corresponding Page object. This can happen if a page is deleted, + # because PageLogEntry records are kept on deletion. self.login(username="alice", password="password") + self.change_something("Alice's edit") + self.child_page.delete() response = self.client.get(reverse("wagtailadmin_home")) self.assertEqual(response.status_code, 200) @@ -102,7 +100,11 @@ class TestRecentEditsPanel(WagtailTestUtils, TestCase): """Test if the panel actually returns expected pages""" self.login(username="bob", password="password") # change a page - self.change_something("Bob's edit") + + edit_timestamp = timezone.now() + with freeze_time(edit_timestamp): + self.change_something("Bob's edit") + # set a user to 'mock' a request self.client.user = get_user_model().objects.get(email="bob@example.com") # get the panel to get the last edits @@ -111,11 +113,36 @@ class TestRecentEditsPanel(WagtailTestUtils, TestCase): page = Page.objects.get(pk=self.child_page.id).specific - # check if the revision is the revision of edited Page - self.assertEqual(ctx["last_edits"][0][0].content_object, page) - # check if the page in this list is the specific page of this revision + # check the timestamp matches the edit + self.assertEqual(ctx["last_edits"][0][0], edit_timestamp) + # check if the page in this list is the specific page self.assertEqual(ctx["last_edits"][0][1], page) + def test_copying_does_not_count_as_an_edit(self): + self.login(username="bob", password="password") + # change a page + self.change_something("Bob was ere") + + # copy the page + post_data = { + "new_title": "Goodbye world!", + "new_slug": "goodbye-world", + "new_parent_page": str(self.root_page.id), + "copy_subpages": False, + "alias": False, + } + self.client.post( + reverse("wagtailadmin_pages:copy", args=(self.child_page.id,)), post_data + ) + # check that page has been copied + self.assertTrue(Page.objects.get(title="Goodbye world!")) + + response = self.client.get(reverse("wagtailadmin_home")) + self.assertEqual(response.status_code, 200) + self.assertContains(response, "Your most recent edits") + self.assertContains(response, "Bob was ere") + self.assertNotContains(response, "Goodbye world!") + class TestRecentEditsQueryCount(WagtailTestUtils, TestCase): fixtures = ["test.json"] @@ -128,7 +155,7 @@ class TestRecentEditsQueryCount(WagtailTestUtils, TestCase): # an unpredictable number of queries) pages_to_edit = Page.objects.filter(id__in=[4, 5, 6, 9, 12, 13]).specific() for page in pages_to_edit: - page.save_revision(user=self.bob) + page.save_revision(user=self.bob, log_action=True) def test_panel_query_count(self): # fake a request object with bob as the user diff --git a/wagtail/admin/views/home.py b/wagtail/admin/views/home.py index 4a05dce3bf..44ea3fc913 100644 --- a/wagtail/admin/views/home.py +++ b/wagtail/admin/views/home.py @@ -5,7 +5,6 @@ from typing import Any, Mapping, Union from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.auth.decorators import permission_required -from django.db import connection from django.db.models import Exists, IntegerField, Max, OuterRef, Q from django.db.models.functions import Cast from django.forms import Media @@ -21,6 +20,7 @@ from wagtail.admin.ui.components import Component from wagtail.admin.views.generic import WagtailAdminTemplateMixin from wagtail.models import ( Page, + PageLogEntry, Revision, TaskState, WorkflowState, @@ -255,44 +255,26 @@ class RecentEditsPanel(Component): # Last n edited pages edit_count = getattr(settings, "WAGTAILADMIN_RECENT_EDITS_LIMIT", 5) - if connection.vendor == "mysql": - # MySQL can't handle the subselect created by the ORM version - - # it fails with "This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'" - last_edits = Revision.objects.raw( - """ - SELECT wr.* FROM - wagtailcore_revision wr JOIN ( - SELECT max(created_at) AS max_created_at, object_id FROM - wagtailcore_revision WHERE user_id = %s AND base_content_type_id = %s GROUP BY object_id ORDER BY max_created_at DESC LIMIT %s - ) AS max_rev ON max_rev.max_created_at = wr.created_at ORDER BY wr.created_at DESC - """, - [ - User._meta.pk.get_db_prep_value(request.user.pk, connection), - get_default_page_content_type().id, - edit_count, - ], - ) - else: - last_edits_dates = ( - Revision.page_revisions.filter(user=request.user) - .values("object_id") - .annotate(latest_date=Max("created_at")) - .order_by("-latest_date") - .values("latest_date")[:edit_count] - ) - last_edits = Revision.page_revisions.filter( - created_at__in=last_edits_dates - ).order_by("-created_at") - # The revision's object_id is a string, so cast it to int first. - page_keys = [int(pr.object_id) for pr in last_edits] - pages = Page.objects.specific().in_bulk(page_keys) - context["last_edits"] = [] - for revision in last_edits: - page = pages.get(int(revision.object_id)) + # Query the audit log to get a resultset of (page ID, latest edit timestamp) + last_edits_dates = ( + PageLogEntry.objects.filter(user=request.user, action="wagtail.edit") + .values("page_id") + .annotate(latest_date=Max("timestamp")) + .order_by("-latest_date")[:edit_count] + ) + # Retrieve the page objects for those IDs + pages_mapping = Page.objects.specific().in_bulk( + [log["page_id"] for log in last_edits_dates] + ) + # Compile a list of (latest edit timestamp, page object) tuples + last_edits = [] + for log in last_edits_dates: + page = pages_mapping.get(log["page_id"]) if page: - context["last_edits"].append([revision, page]) + last_edits.append((log["latest_date"], page)) + context["last_edits"] = last_edits context["request"] = request return context