From 2a785f606fb196dd6937013a571bbac9b386b59e Mon Sep 17 00:00:00 2001 From: Neil Muller Date: Fri, 5 Jun 2026 14:25:16 +0200 Subject: [PATCH 1/3] Drop use of _reversion_order_version_queryset Using this was always a bit of a hack, and it changed signature in recent reversion versions, breaking our list page. Since we don't actually use the features it provided over just pulling the raw queryset, simplying this is a simple fix that keeps compatability (and should be more robust since we're not using an internal method) --- wafer/compare/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wafer/compare/admin.py b/wafer/compare/admin.py index a2248124..2b0bc58e 100644 --- a/wafer/compare/admin.py +++ b/wafer/compare/admin.py @@ -162,9 +162,9 @@ def comparelist_view(self, request, object_id, extra_context=None): { "revision": version.revision, "url": reverse("%s:%s_%s_compare" % (self.admin_site.name, opts.app_label, opts.model_name), args=(quote(version.object_id), version.id)), - } for version in self._reversion_order_version_queryset(Version.objects.get_for_object_reference( + } for version in Version.objects.get_for_object_reference( self.model, - object_id).select_related("revision__user"))] + object_id).select_related("revision__user")] context = {"action_list": action_list, "opts": opts, "object_id": quote(object_id), From fcb1e10fa75b2f0c0e7bba18a74443ac806350c3 Mon Sep 17 00:00:00 2001 From: Neil Muller Date: Sat, 6 Jun 2026 15:19:00 +0200 Subject: [PATCH 2/3] Add minimal tests for the compare view --- wafer/compare/tests/__init__.py | 0 .../compare/tests/test_wafer_compare_list.py | 53 +++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 wafer/compare/tests/__init__.py create mode 100644 wafer/compare/tests/test_wafer_compare_list.py diff --git a/wafer/compare/tests/__init__.py b/wafer/compare/tests/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/wafer/compare/tests/test_wafer_compare_list.py b/wafer/compare/tests/test_wafer_compare_list.py new file mode 100644 index 00000000..7ed947f5 --- /dev/null +++ b/wafer/compare/tests/test_wafer_compare_list.py @@ -0,0 +1,53 @@ +# This tests the basic compare list, to ensure it's working as expected + +from django.contrib.auth import get_user_model +from django.utils.timezone import datetime, now + +from django.test import Client, TestCase + +from reversion import revisions + +from wafer.talks.models import Talk, TalkType, SUBMITTED +from wafer.talks.tests.fixtures import create_talk +from wafer.tests.utils import create_user + + +class TestBasicCompareList(TestCase): + """Basic talk tests""" + + def setUp(self): + """Setup a user with a talk""" + talk_user = create_user('john') + self.super = create_user('super', superuser=True) + self.talk_a = create_talk('This is a test talk', status=SUBMITTED, user=talk_user) + # Create a base revision + with revisions.create_revision(): + self.talk_a.save() + # Edit 1 + self.talk_a.abstract = "This is an abstract" + with revisions.create_revision(): + self.talk_a.save() + self.talk_a.abstract = "This is not an abstract" + with revisions.create_revision(): + self.talk_a.save() + self.client = Client() + + def test_get_compare_list(self): + """Get the compare list and check the number of entries""" + self.client.login(username="super", password="super_password") + response = self.client.get(f'/admin/talks/talk/{self.talk_a.pk}/comparelist/') + # Check we have 3 revisions to compare + self.assertIn(b'/1/compare', response.content) + self.assertIn(b'/2/compare', response.content) + self.assertIn(b'/3/compare', response.content) + # Check that we don't have unexpcted ones + self.assertNotIn(b'/4/compare', response.content) + + def test_get_diffs(self): + """Check that diffs look sensible""" + self.client.login(username="super", password="super_password") + response = self.client.get(f'/admin/talks/talk/{self.talk_a.pk}/2/compare/') + # Check that the 'not' we added is marked + # This should maybe a regex to avoid assumptions about the whitespace + # positioning. + self.assertIn(b'>not ', response.content) From 77faa1dab614685c467ab349ad3cabe39b84e87d Mon Sep 17 00:00:00 2001 From: Neil Muller Date: Sun, 7 Jun 2026 13:28:51 +0200 Subject: [PATCH 3/3] Refactor test to work with postgresql/sqlite differences --- .../compare/tests/test_wafer_compare_list.py | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/wafer/compare/tests/test_wafer_compare_list.py b/wafer/compare/tests/test_wafer_compare_list.py index 7ed947f5..8fa9b8c8 100644 --- a/wafer/compare/tests/test_wafer_compare_list.py +++ b/wafer/compare/tests/test_wafer_compare_list.py @@ -1,5 +1,7 @@ # This tests the basic compare list, to ensure it's working as expected +import re + from django.contrib.auth import get_user_model from django.utils.timezone import datetime, now @@ -31,22 +33,26 @@ def setUp(self): with revisions.create_revision(): self.talk_a.save() self.client = Client() + # We use an re here as the revision numbers aren't guaranteed to be stable + # across different databases + self.compare_re = re.compile('/admin/talks/talk/[0-9]+/[0-9]+/compare/') def test_get_compare_list(self): """Get the compare list and check the number of entries""" self.client.login(username="super", password="super_password") response = self.client.get(f'/admin/talks/talk/{self.talk_a.pk}/comparelist/') - # Check we have 3 revisions to compare - self.assertIn(b'/1/compare', response.content) - self.assertIn(b'/2/compare', response.content) - self.assertIn(b'/3/compare', response.content) - # Check that we don't have unexpcted ones - self.assertNotIn(b'/4/compare', response.content) + # Check we have exactly 3 revisions to compare + results = self.compare_re.findall(response.content.decode()) + self.assertEqual(len(results), 3) def test_get_diffs(self): """Check that diffs look sensible""" self.client.login(username="super", password="super_password") - response = self.client.get(f'/admin/talks/talk/{self.talk_a.pk}/2/compare/') + response = self.client.get(f'/admin/talks/talk/{self.talk_a.pk}/comparelist/') + results = self.compare_re.findall(response.content.decode()) + # we grab the second to look at the abstract changes + url = results[1] + response = self.client.get(url) # Check that the 'not' we added is marked # This should maybe a regex to avoid assumptions about the whitespace # positioning.