From 232f810249a89773fd6f17f2cc9d2b320e56fa2b Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:19:02 +0200 Subject: [PATCH 01/13] feat: add object_id_field option to register() --- reversion/revisions.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/reversion/revisions.py b/reversion/revisions.py index 5e2ed4f6..c6c45cad 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -21,6 +21,7 @@ "for_concrete_model", "ignore_duplicates", "use_natural_foreign_keys", + "object_id_field" )) @@ -168,7 +169,10 @@ def _add_to_revision(obj, using, model_db, explicit): return version_options = _get_options(obj.__class__) content_type = _get_content_type(obj.__class__, using) - object_id = force_str(obj.pk) + if version_options.object_id_field: + object_id = force_str(getattr(obj, version_options.object_id_field)) + else: + object_id = force_str(obj.pk) version_key = (content_type, object_id) # If the obj is already in the revision, stop now. db_versions = _current_frame().db_versions @@ -191,7 +195,7 @@ def _add_to_revision(obj, using, model_db, explicit): ) # If the version is a duplicate, stop now. if version_options.ignore_duplicates and explicit: - previous_version = Version.objects.using(using).get_for_object(obj, model_db=model_db).first() + previous_version = Version.objects.using(using).get_for_object_reference(obj.__class__, object_id, model_db=model_db).first() if previous_version and previous_version._local_field_dict == version._local_field_dict: return # Store the version. @@ -209,6 +213,10 @@ def add_to_revision(obj, model_db=None): _add_to_revision(obj, db, model_db, True) +def _get_object_id_field(model): + return (_get_options(model).object_id_field or "pk") if is_registered(model) else "pk" + + def _save_revision(versions, user=None, comment="", meta=(), date_created=None, using=None): from reversion.models import Revision from reversion.models import Version @@ -221,7 +229,7 @@ def _save_revision(versions, user=None, comment="", meta=(), date_created=None, model: { db: frozenset(map( force_str, - model._base_manager.using(db).filter(pk__in=pks).values_list("pk", flat=True), + model._base_manager.using(db).filter(**{f"{_get_object_id_field(model)}__in": pks}).values_list(_get_object_id_field(model), flat=True), )) for db, pks in db_pks.items() } @@ -376,7 +384,7 @@ def _get_senders_and_signals(model): def register(model=None, fields=None, exclude=(), follow=(), format="json", - for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False): + for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False, object_id_field=None): def register(model): # Prevent multiple registration. if is_registered(model): @@ -401,6 +409,7 @@ def register(model): for_concrete_model=for_concrete_model, ignore_duplicates=ignore_duplicates, use_natural_foreign_keys=use_natural_foreign_keys, + object_id_field=object_id_field, ) # Register the model. _registered_models[_get_registration_key(model)] = version_options From fe504ee82221abdcaea8f23e8e2a1738f9de2a50 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:19:10 +0200 Subject: [PATCH 02/13] fix: support object_id_field in Revision.revert() and get_for_object() --- reversion/models.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/reversion/models.py b/reversion/models.py index c259a686..9e04349b 100644 --- a/reversion/models.py +++ b/reversion/models.py @@ -20,7 +20,7 @@ from reversion.errors import RevertError from reversion.revisions import (_follow_relations_recursive, - _get_content_type, _get_options) + _get_content_type, _get_options, _get_object_id_field, is_registered) logger = logging.getLogger(__name__) @@ -91,7 +91,8 @@ def revert(self, delete=False): model = version._model try: # Load the model instance from the same DB as it was saved under. - old_revision.add(model._default_manager.using(version.db).get(pk=version.object_id)) + id_field = _get_object_id_field(model) + old_revision.add(model._default_manager.using(version.db).get(**{id_field: version.object_id})) except model.DoesNotExist: pass # Calculate the set of all objects that are in the revision now. @@ -135,6 +136,12 @@ def get_for_object_reference(self, model, object_id, model_db=None): ) def get_for_object(self, obj, model_db=None): + if is_registered(obj.__class__): + opts = _get_options(obj.__class__) + if opts.object_id_field: + return self.get_for_object_reference( + obj.__class__, getattr(obj, opts.object_id_field), model_db=model_db + ) return self.get_for_object_reference(obj.__class__, obj.pk, model_db=model_db) def get_deleted(self, model, model_db=None): From 6dbd423d94119abf865fb42dc98d98627e0332c7 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:19:15 +0200 Subject: [PATCH 03/13] feat: add VersionAdmin hooks for object_id_field support --- reversion/admin.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/reversion/admin.py b/reversion/admin.py index 8b9f17d2..241ee368 100644 --- a/reversion/admin.py +++ b/reversion/admin.py @@ -48,6 +48,22 @@ def reversion_register(self, model, **kwargs): """Registers the model with reversion.""" register(model, **kwargs) + def get_reversion_object_id(self, request, object_id): + """Returns the object_id used in version storage for the given admin URL object_id. + + Override this when using object_id_field in register() to translate + the admin URL pk to the custom field value used for version storage. + """ + return unquote(object_id) + + def get_reversion_changeform_object_id(self, version): + """Returns the pk-based object_id to pass to changeform_view for a given version. + + Override this when using object_id_field in register() to translate + the stored object_id back to the pk that Django admin expects. + """ + return quote(version.object_id) + def get_version_ordering(self, request): """Hook for specifying custom field ordering for the version queryset.""" # Default ordering logic uses version ID only @@ -200,7 +216,7 @@ def _reversion_revisionform_view(self, request, version, template_name, extra_co version.revision.revert(delete=True) # Run the normal changeform view. with self.create_revision(request): - response = self.changeform_view(request, quote(version.object_id), request.path, extra_context) + response = self.changeform_view(request, self.get_reversion_changeform_object_id(version), request.path, extra_context) # Decide on whether the keep the changes. if request.method == "POST" and response.status_code == 302: set_comment(_("Reverted to previous version, saved on %(datetime)s") % { @@ -317,7 +333,7 @@ def history_view(self, request, object_id, extra_context=None): for version in self._reversion_order_version_queryset(request, Version.objects.get_for_object_reference( self.model, - unquote(object_id), # Underscores in primary key get quoted to "_5F" + self.get_reversion_object_id(request, object_id), ).select_related("revision", "revision__user")) ] # Compile the context. From 31151f9dc4dbd0528f64fb2358bbf4d8dd8bff67 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:19:19 +0200 Subject: [PATCH 04/13] test: add TestModelCustomObjectId model and migration --- .../0003_testmodelcustomobjectid.py | 21 +++++++++++++++++++ tests/test_app/models.py | 4 ++++ 2 files changed, 25 insertions(+) create mode 100644 tests/test_app/migrations/0003_testmodelcustomobjectid.py diff --git a/tests/test_app/migrations/0003_testmodelcustomobjectid.py b/tests/test_app/migrations/0003_testmodelcustomobjectid.py new file mode 100644 index 00000000..37ea99ef --- /dev/null +++ b/tests/test_app/migrations/0003_testmodelcustomobjectid.py @@ -0,0 +1,21 @@ +# Generated by Django 6.0.4 on 2026-05-02 12:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('test_app', '0002_alter_testmodel_related_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='TestModelCustomObjectId', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('slug', models.CharField(max_length=191, unique=True)), + ('name', models.CharField(default='v1', max_length=191)), + ], + ), + ] diff --git a/tests/test_app/models.py b/tests/test_app/models.py index bfb3a16c..ccc4f395 100644 --- a/tests/test_app/models.py +++ b/tests/test_app/models.py @@ -150,3 +150,7 @@ class TestModelWithUniqueConstraint(models.Model): max_length=191, unique=True, ) + +class TestModelCustomObjectId(models.Model): + slug = models.CharField(max_length=191, unique=True) + name = models.CharField(max_length=191, default="v1") From 80a116f4a15c84d489c3faaa76d883619456989b Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:19:23 +0200 Subject: [PATCH 05/13] test: add tests and admin example for object_id_field --- tests/test_app/admin.py | 20 +++++++++- tests/test_app/tests/test_admin.py | 61 ++++++++++++++++++++++++++++- tests/test_app/tests/test_models.py | 58 +++++++++++++++++++++++++++ 3 files changed, 136 insertions(+), 3 deletions(-) diff --git a/tests/test_app/admin.py b/tests/test_app/admin.py index c6641231..b9b549d9 100644 --- a/tests/test_app/admin.py +++ b/tests/test_app/admin.py @@ -1,6 +1,8 @@ from django.contrib import admin +from django.contrib.admin.utils import unquote, quote +from django.utils.encoding import force_str from reversion.admin import VersionAdmin -from test_app.models import TestModel, TestModelRelated +from test_app.models import TestModel, TestModelRelated, TestModelCustomObjectId class TestModelAdmin(VersionAdmin): @@ -8,7 +10,21 @@ class TestModelAdmin(VersionAdmin): filter_horizontal = ("related",) -admin.site.register(TestModel, TestModelAdmin) +class TestModelCustomObjectIdAdmin(VersionAdmin): + + def reversion_register(self, model, **kwargs): + kwargs["object_id_field"] = "slug" + super().reversion_register(model, **kwargs) + + def get_reversion_object_id(self, request, object_id): + obj = self.get_object(request, unquote(object_id)) + return force_str(obj.slug) if obj else unquote(object_id) + def get_reversion_changeform_object_id(self, version): + obj = self.model._default_manager.using(version.db).get(slug=version.object_id) + return quote(str(obj.pk)) + +admin.site.register(TestModel, TestModelAdmin) admin.site.register(TestModelRelated, admin.ModelAdmin) +admin.site.register(TestModelCustomObjectId, TestModelCustomObjectIdAdmin) diff --git a/tests/test_app/tests/test_admin.py b/tests/test_app/tests/test_admin.py index 8d992bf8..b35b6422 100644 --- a/tests/test_app/tests/test_admin.py +++ b/tests/test_app/tests/test_admin.py @@ -9,7 +9,7 @@ import reversion from reversion.admin import VersionAdmin from reversion.models import Version -from test_app.models import TestModel, TestModelParent, TestModelInline, TestModelGenericInline, TestModelEscapePK +from test_app.models import TestModel, TestModelParent, TestModelInline, TestModelGenericInline, TestModelEscapePK, TestModelCustomObjectId from test_app.tests.base import TestBase, LoginMixin @@ -569,3 +569,62 @@ def testAutoRegisterInline(self): def testAutoRegisterGenericInline(self): self.assertTrue(reversion.is_registered(TestModelGenericInline)) + + +class CustomObjectIdAdminMixin(TestBase): + + def setUp(self): + super().setUp() + reversion.register(TestModelCustomObjectId, object_id_field="slug") + self.reloadUrls() + + def tearDown(self): + super().tearDown() + self.reloadUrls() + + +class CustomObjectIdAdminHistoryViewTest(LoginMixin, CustomObjectIdAdminMixin, TestBase): + + def testHistoryView(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug="test", name="v1") + with reversion.create_revision(): + obj.name = "v2" + obj.save() + response = self.client.get(resolve_url("admin:test_app_testmodelcustomobjectid_history", obj.pk)) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.context["action_list"]), 2) + + +class CustomObjectIdAdminRevisionViewTest(LoginMixin, CustomObjectIdAdminMixin, TestBase): + + def setUp(self): + super().setUp() + with reversion.create_revision(): + self.obj = TestModelCustomObjectId.objects.create(slug="test", name="v1") + with reversion.create_revision(): + self.obj.name = "v2" + self.obj.save() + + def testRevisionViewGet(self): + version = Version.objects.get_for_object_reference(TestModelCustomObjectId, "test")[1] + response = self.client.get(resolve_url( + "admin:test_app_testmodelcustomobjectid_revision", + self.obj.slug, + version.pk, + )) + self.assertEqual(response.status_code, 200) + self.assertContains(response, 'value="v1"') + # Verify the revert was rolled back (GET should not persist changes). + self.obj.refresh_from_db() + self.assertEqual(self.obj.name, "v2") + + def testRevisionViewRevert(self): + version = Version.objects.get_for_object_reference(TestModelCustomObjectId, "test")[1] + self.client.post(resolve_url( + "admin:test_app_testmodelcustomobjectid_revision", + self.obj.slug, + version.pk, + ), {"slug": "test", "name": "v1"}) + self.obj.refresh_from_db() + self.assertEqual(self.obj.name, "v1") diff --git a/tests/test_app/tests/test_models.py b/tests/test_app/tests/test_models.py index 2d2ca0a9..cd065d18 100644 --- a/tests/test_app/tests/test_models.py +++ b/tests/test_app/tests/test_models.py @@ -5,6 +5,7 @@ TestModelNestedInline, TestModelInlineByNaturalKey, TestModelWithNaturalKey, TestModelWithUniqueConstraint, + TestModelCustomObjectId ) from test_app.tests.base import TestBase, TestModelMixin, TestModelParentMixin import json @@ -458,3 +459,60 @@ def testTransactionInRollbackState(self): TestModelWithUniqueConstraint.objects.create(name='A') except Exception: pass + +class CustomObjectIdTest(TestBase): + + def setUp(self): + super().setUp() + reversion.register(TestModelCustomObjectId, object_id_field='slug') + + def testObjectIdStoredAsSlug(self): + with reversion.create_revision(): + TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + version = Version.objects.get_for_object_reference(TestModelCustomObjectId, 'custom-id').get() + self.assertEqual(version.object_id, 'custom-id') + + def testGetForObject(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + self.assertEqual(Version.objects.get_for_object(obj).count(), 1) + + def testFieldDict(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + version = Version.objects.get_for_object_reference(TestModelCustomObjectId, 'custom-id').get() + self.assertEqual(version.field_dict, {'id': obj.pk, 'slug': 'custom-id', 'name': 'v1'}) + + def testMultipleVersions(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + with reversion.create_revision(): + obj.name = 'v2' + obj.save() + versions = Version.objects.get_for_object_reference(TestModelCustomObjectId, 'custom-id') + self.assertEqual(versions.count(), 2) + self.assertEqual(versions[0].field_dict['name'], 'v2') + self.assertEqual(versions[1].field_dict['name'], 'v1') + + +class CustomObjectIdIgnoreDuplicatesTest(TestBase): + + def setUp(self): + reversion.register(TestModelCustomObjectId, object_id_field='slug', ignore_duplicates=True) + + def testIgnoreDuplicates(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + with reversion.create_revision(): + obj.save() + versions = Version.objects.get_for_object_reference(TestModelCustomObjectId, 'custom-id') + self.assertEqual(versions.count(), 1) + + def testIgnoreDuplicatesNewData(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + with reversion.create_revision(): + obj.name = 'v2' + obj.save() + versions = Version.objects.get_for_object_reference(TestModelCustomObjectId, 'custom-id') + self.assertEqual(versions.count(), 2) From eedcbef300e2f9e2b2b2afc6c3c59819586940d5 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:28:43 +0200 Subject: [PATCH 06/13] style: fix flake8 line length and blank line issues --- reversion/admin.py | 4 +++- reversion/models.py | 4 +++- reversion/revisions.py | 8 ++++++-- tests/test_app/models.py | 1 + tests/test_app/tests/test_admin.py | 4 +++- tests/test_app/tests/test_models.py | 1 + 6 files changed, 17 insertions(+), 5 deletions(-) diff --git a/reversion/admin.py b/reversion/admin.py index 241ee368..adf157a4 100644 --- a/reversion/admin.py +++ b/reversion/admin.py @@ -216,7 +216,9 @@ def _reversion_revisionform_view(self, request, version, template_name, extra_co version.revision.revert(delete=True) # Run the normal changeform view. with self.create_revision(request): - response = self.changeform_view(request, self.get_reversion_changeform_object_id(version), request.path, extra_context) + response = self.changeform_view( + request, self.get_reversion_changeform_object_id(version), request.path, extra_context + ) # Decide on whether the keep the changes. if request.method == "POST" and response.status_code == 302: set_comment(_("Reverted to previous version, saved on %(datetime)s") % { diff --git a/reversion/models.py b/reversion/models.py index 9e04349b..8f9e8b77 100644 --- a/reversion/models.py +++ b/reversion/models.py @@ -92,7 +92,9 @@ def revert(self, delete=False): try: # Load the model instance from the same DB as it was saved under. id_field = _get_object_id_field(model) - old_revision.add(model._default_manager.using(version.db).get(**{id_field: version.object_id})) + old_revision.add( + model._default_manager.using(version.db).get(**{id_field: version.object_id}) + ) except model.DoesNotExist: pass # Calculate the set of all objects that are in the revision now. diff --git a/reversion/revisions.py b/reversion/revisions.py index c6c45cad..b57757ac 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -195,7 +195,9 @@ def _add_to_revision(obj, using, model_db, explicit): ) # If the version is a duplicate, stop now. if version_options.ignore_duplicates and explicit: - previous_version = Version.objects.using(using).get_for_object_reference(obj.__class__, object_id, model_db=model_db).first() + previous_version = Version.objects.using(using).get_for_object_reference( + obj.__class__, object_id, model_db=model_db + ).first() if previous_version and previous_version._local_field_dict == version._local_field_dict: return # Store the version. @@ -229,7 +231,9 @@ def _save_revision(versions, user=None, comment="", meta=(), date_created=None, model: { db: frozenset(map( force_str, - model._base_manager.using(db).filter(**{f"{_get_object_id_field(model)}__in": pks}).values_list(_get_object_id_field(model), flat=True), + model._base_manager.using(db).filter( + **{f"{_get_object_id_field(model)}__in": pks} + ).values_list(_get_object_id_field(model), flat=True), )) for db, pks in db_pks.items() } diff --git a/tests/test_app/models.py b/tests/test_app/models.py index ccc4f395..b404b08d 100644 --- a/tests/test_app/models.py +++ b/tests/test_app/models.py @@ -151,6 +151,7 @@ class TestModelWithUniqueConstraint(models.Model): unique=True, ) + class TestModelCustomObjectId(models.Model): slug = models.CharField(max_length=191, unique=True) name = models.CharField(max_length=191, default="v1") diff --git a/tests/test_app/tests/test_admin.py b/tests/test_app/tests/test_admin.py index b35b6422..c05a092f 100644 --- a/tests/test_app/tests/test_admin.py +++ b/tests/test_app/tests/test_admin.py @@ -9,7 +9,9 @@ import reversion from reversion.admin import VersionAdmin from reversion.models import Version -from test_app.models import TestModel, TestModelParent, TestModelInline, TestModelGenericInline, TestModelEscapePK, TestModelCustomObjectId +from test_app.models import ( + TestModel, TestModelParent, TestModelInline, TestModelGenericInline, TestModelEscapePK, TestModelCustomObjectId +) from test_app.tests.base import TestBase, LoginMixin diff --git a/tests/test_app/tests/test_models.py b/tests/test_app/tests/test_models.py index cd065d18..9ea27004 100644 --- a/tests/test_app/tests/test_models.py +++ b/tests/test_app/tests/test_models.py @@ -460,6 +460,7 @@ def testTransactionInRollbackState(self): except Exception: pass + class CustomObjectIdTest(TestBase): def setUp(self): From 9d0936fce0b2a0d9cdb313a47d8e86eca6c9d47d Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Sat, 2 May 2026 15:47:49 +0200 Subject: [PATCH 07/13] test: move CustomObjectId admin registration to setUp/tearDown --- tests/test_app/admin.py | 20 +------------------- tests/test_app/tests/test_admin.py | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/tests/test_app/admin.py b/tests/test_app/admin.py index b9b549d9..00874c43 100644 --- a/tests/test_app/admin.py +++ b/tests/test_app/admin.py @@ -1,8 +1,6 @@ from django.contrib import admin -from django.contrib.admin.utils import unquote, quote -from django.utils.encoding import force_str from reversion.admin import VersionAdmin -from test_app.models import TestModel, TestModelRelated, TestModelCustomObjectId +from test_app.models import TestModel, TestModelRelated class TestModelAdmin(VersionAdmin): @@ -10,21 +8,5 @@ class TestModelAdmin(VersionAdmin): filter_horizontal = ("related",) -class TestModelCustomObjectIdAdmin(VersionAdmin): - - def reversion_register(self, model, **kwargs): - kwargs["object_id_field"] = "slug" - super().reversion_register(model, **kwargs) - - def get_reversion_object_id(self, request, object_id): - obj = self.get_object(request, unquote(object_id)) - return force_str(obj.slug) if obj else unquote(object_id) - - def get_reversion_changeform_object_id(self, version): - obj = self.model._default_manager.using(version.db).get(slug=version.object_id) - return quote(str(obj.pk)) - - admin.site.register(TestModel, TestModelAdmin) admin.site.register(TestModelRelated, admin.ModelAdmin) -admin.site.register(TestModelCustomObjectId, TestModelCustomObjectIdAdmin) diff --git a/tests/test_app/tests/test_admin.py b/tests/test_app/tests/test_admin.py index c05a092f..356ec289 100644 --- a/tests/test_app/tests/test_admin.py +++ b/tests/test_app/tests/test_admin.py @@ -573,15 +573,31 @@ def testAutoRegisterGenericInline(self): self.assertTrue(reversion.is_registered(TestModelGenericInline)) +class TestModelCustomObjectIdAdmin(VersionAdmin): + + def reversion_register(self, model, **kwargs): + kwargs["object_id_field"] = "slug" + super().reversion_register(model, **kwargs) + + def get_reversion_object_id(self, request, object_id): + obj = self.get_object(request, admin.utils.unquote(object_id)) + return str(obj.slug) if obj else admin.utils.unquote(object_id) + + def get_reversion_changeform_object_id(self, version): + obj = self.model._default_manager.using(version.db).get(slug=version.object_id) + return admin.utils.quote(str(obj.pk)) + + class CustomObjectIdAdminMixin(TestBase): def setUp(self): super().setUp() - reversion.register(TestModelCustomObjectId, object_id_field="slug") + admin.site.register(TestModelCustomObjectId, TestModelCustomObjectIdAdmin) self.reloadUrls() def tearDown(self): super().tearDown() + admin.site.unregister(TestModelCustomObjectId) self.reloadUrls() From d3fbf845432b02d74b88d4917b85498954fa796f Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Mon, 4 May 2026 15:35:10 +0200 Subject: [PATCH 08/13] fix: refactored code to simplify logic --- reversion/admin.py | 28 +++++++++------------------- reversion/models.py | 8 ++++---- reversion/revisions.py | 10 ++++------ tests/test_app/tests/test_admin.py | 8 -------- 4 files changed, 17 insertions(+), 37 deletions(-) diff --git a/reversion/admin.py b/reversion/admin.py index adf157a4..269ad770 100644 --- a/reversion/admin.py +++ b/reversion/admin.py @@ -18,7 +18,7 @@ from reversion.errors import RevertError from reversion.models import Version -from reversion.revisions import is_active, register, is_registered, set_comment, create_revision, set_user +from reversion.revisions import is_active, register, is_registered, set_comment, create_revision, set_user, _get_options from reversion.utils import mute_signals @@ -48,22 +48,6 @@ def reversion_register(self, model, **kwargs): """Registers the model with reversion.""" register(model, **kwargs) - def get_reversion_object_id(self, request, object_id): - """Returns the object_id used in version storage for the given admin URL object_id. - - Override this when using object_id_field in register() to translate - the admin URL pk to the custom field value used for version storage. - """ - return unquote(object_id) - - def get_reversion_changeform_object_id(self, version): - """Returns the pk-based object_id to pass to changeform_view for a given version. - - Override this when using object_id_field in register() to translate - the stored object_id back to the pk that Django admin expects. - """ - return quote(version.object_id) - def get_version_ordering(self, request): """Hook for specifying custom field ordering for the version queryset.""" # Default ordering logic uses version ID only @@ -216,8 +200,12 @@ def _reversion_revisionform_view(self, request, version, template_name, extra_co version.revision.revert(delete=True) # Run the normal changeform view. with self.create_revision(request): + obj = get_object_or_404( + version._model._default_manager.using(version.db), + **{_get_options(version._model).object_id_field: version.object_id}, + ) response = self.changeform_view( - request, self.get_reversion_changeform_object_id(version), request.path, extra_context + request, quote(str(obj.pk)), request.path, extra_context ) # Decide on whether the keep the changes. if request.method == "POST" and response.status_code == 302: @@ -323,6 +311,8 @@ def history_view(self, request, object_id, extra_context=None): if not self.has_change_permission(request): raise PermissionDenied + obj = get_object_or_404(self.model, pk=unquote(object_id)) + reversion_object_id = str(getattr(obj, _get_options(self.model).object_id_field)) opts = self.model._meta action_list = [ { @@ -335,7 +325,7 @@ def history_view(self, request, object_id, extra_context=None): for version in self._reversion_order_version_queryset(request, Version.objects.get_for_object_reference( self.model, - self.get_reversion_object_id(request, object_id), + reversion_object_id, ).select_related("revision", "revision__user")) ] # Compile the context. diff --git a/reversion/models.py b/reversion/models.py index 8f9e8b77..0e1a2fa8 100644 --- a/reversion/models.py +++ b/reversion/models.py @@ -140,10 +140,10 @@ def get_for_object_reference(self, model, object_id, model_db=None): def get_for_object(self, obj, model_db=None): if is_registered(obj.__class__): opts = _get_options(obj.__class__) - if opts.object_id_field: - return self.get_for_object_reference( - obj.__class__, getattr(obj, opts.object_id_field), model_db=model_db - ) + return self.get_for_object_reference( + obj.__class__, getattr(obj, opts.object_id_field), model_db=model_db + ) + return self.get_for_object_reference(obj.__class__, obj.pk, model_db=model_db) def get_deleted(self, model, model_db=None): diff --git a/reversion/revisions.py b/reversion/revisions.py index b57757ac..e0cf43a1 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -169,10 +169,8 @@ def _add_to_revision(obj, using, model_db, explicit): return version_options = _get_options(obj.__class__) content_type = _get_content_type(obj.__class__, using) - if version_options.object_id_field: - object_id = force_str(getattr(obj, version_options.object_id_field)) - else: - object_id = force_str(obj.pk) + object_id = force_str(getattr(obj, version_options.object_id_field)) + version_key = (content_type, object_id) # If the obj is already in the revision, stop now. db_versions = _current_frame().db_versions @@ -216,7 +214,7 @@ def add_to_revision(obj, model_db=None): def _get_object_id_field(model): - return (_get_options(model).object_id_field or "pk") if is_registered(model) else "pk" + return (_get_options(model).object_id_field or "pk") def _save_revision(versions, user=None, comment="", meta=(), date_created=None, using=None): @@ -388,7 +386,7 @@ def _get_senders_and_signals(model): def register(model=None, fields=None, exclude=(), follow=(), format="json", - for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False, object_id_field=None): + for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False, object_id_field="pk"): def register(model): # Prevent multiple registration. if is_registered(model): diff --git a/tests/test_app/tests/test_admin.py b/tests/test_app/tests/test_admin.py index 356ec289..0d89e8ba 100644 --- a/tests/test_app/tests/test_admin.py +++ b/tests/test_app/tests/test_admin.py @@ -579,14 +579,6 @@ def reversion_register(self, model, **kwargs): kwargs["object_id_field"] = "slug" super().reversion_register(model, **kwargs) - def get_reversion_object_id(self, request, object_id): - obj = self.get_object(request, admin.utils.unquote(object_id)) - return str(obj.slug) if obj else admin.utils.unquote(object_id) - - def get_reversion_changeform_object_id(self, version): - obj = self.model._default_manager.using(version.db).get(slug=version.object_id) - return admin.utils.quote(str(obj.pk)) - class CustomObjectIdAdminMixin(TestBase): From e13bb700f40b9e0add1b4b324e359d212df18d75 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Mon, 4 May 2026 16:13:39 +0200 Subject: [PATCH 09/13] fix: apply object_id_field to get_deleted and get_for_object --- reversion/models.py | 27 +++++++++++++-------------- reversion/revisions.py | 3 ++- tests/test_app/tests/test_models.py | 13 +++++++++++++ 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/reversion/models.py b/reversion/models.py index 0e1a2fa8..ee8d7e75 100644 --- a/reversion/models.py +++ b/reversion/models.py @@ -20,7 +20,7 @@ from reversion.errors import RevertError from reversion.revisions import (_follow_relations_recursive, - _get_content_type, _get_options, _get_object_id_field, is_registered) + _get_content_type, _get_options, _get_object_id_field) logger = logging.getLogger(__name__) @@ -138,20 +138,17 @@ def get_for_object_reference(self, model, object_id, model_db=None): ) def get_for_object(self, obj, model_db=None): - if is_registered(obj.__class__): - opts = _get_options(obj.__class__) - return self.get_for_object_reference( - obj.__class__, getattr(obj, opts.object_id_field), model_db=model_db - ) - - return self.get_for_object_reference(obj.__class__, obj.pk, model_db=model_db) + opts = _get_options(obj.__class__) + return self.get_for_object_reference( + obj.__class__, getattr(obj, opts.object_id_field), model_db=model_db + ) def get_deleted(self, model, model_db=None): model_db = model_db or router.db_for_write(model) connection = connections[self.db] + object_id_field_name = _get_object_id_field(model) if self.db == model_db and connection.vendor in ("sqlite", "postgresql", "oracle"): - pk_field_name = model._meta.pk.name - object_id_cast_target = model._meta.get_field(pk_field_name) + object_id_cast_target = model._meta.get_field(object_id_field_name) if django.VERSION >= (2, 1): # django 2.0 contains a critical bug that doesn't allow the code below to work, # fallback to casting primary keys then @@ -167,14 +164,14 @@ def get_deleted(self, model, model_db=None): model_qs = ( model._default_manager .using(model_db) - .filter(**{pk_field_name: casted_object_id}) + .filter(**{object_id_field_name: casted_object_id}) ) else: model_qs = ( model._default_manager .using(model_db) - .annotate(_pk_to_object_id=Cast("pk", Version._meta.get_field("object_id"))) - .filter(_pk_to_object_id=models.OuterRef("object_id")) + .annotate(_field_to_object_id=Cast(object_id_field_name, Version._meta.get_field("object_id"))) + .filter(_field_to_object_id=models.OuterRef("object_id")) ) # conditional expressions are being supported since django 3.0 # DISTINCT ON works only for Postgres DB @@ -199,7 +196,9 @@ def get_deleted(self, model, model_db=None): # We have to use a slow subquery. subquery = self.get_for_model(model, model_db=model_db).exclude( object_id__in=list( - model._default_manager.using(model_db).values_list("pk", flat=True).order_by().iterator() + model._default_manager.using(model_db).values_list( + object_id_field_name, flat=True + ).order_by().iterator() ), ).values_list("object_id").annotate( latest_pk=models.Max("pk") diff --git a/reversion/revisions.py b/reversion/revisions.py index e0cf43a1..198310ec 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -214,7 +214,8 @@ def add_to_revision(obj, model_db=None): def _get_object_id_field(model): - return (_get_options(model).object_id_field or "pk") + field = _get_options(model).object_id_field + return model._meta.pk.name if field == "pk" else field def _save_revision(versions, user=None, comment="", meta=(), date_created=None, using=None): diff --git a/tests/test_app/tests/test_models.py b/tests/test_app/tests/test_models.py index 9ea27004..7169751a 100644 --- a/tests/test_app/tests/test_models.py +++ b/tests/test_app/tests/test_models.py @@ -495,6 +495,19 @@ def testMultipleVersions(self): self.assertEqual(versions[0].field_dict['name'], 'v2') self.assertEqual(versions[1].field_dict['name'], 'v1') + def testGetDeleted(self): + with reversion.create_revision(): + obj = TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + obj.delete() + deleted = Version.objects.get_deleted(TestModelCustomObjectId) + self.assertEqual(deleted.count(), 1) + self.assertEqual(deleted[0].object_id, 'custom-id') + + def testGetDeletedNotIncludingExisting(self): + with reversion.create_revision(): + TestModelCustomObjectId.objects.create(slug='custom-id', name='v1') + self.assertEqual(Version.objects.get_deleted(TestModelCustomObjectId).count(), 0) + class CustomObjectIdIgnoreDuplicatesTest(TestBase): From d5c51ae7db3f248b76ecfaaa8d87a495dab24401 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Mon, 4 May 2026 16:17:40 +0200 Subject: [PATCH 10/13] fix: linting error --- reversion/revisions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reversion/revisions.py b/reversion/revisions.py index 198310ec..1060c010 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -170,7 +170,6 @@ def _add_to_revision(obj, using, model_db, explicit): version_options = _get_options(obj.__class__) content_type = _get_content_type(obj.__class__, using) object_id = force_str(getattr(obj, version_options.object_id_field)) - version_key = (content_type, object_id) # If the obj is already in the revision, stop now. db_versions = _current_frame().db_versions From b1e3f6796d7dd09b565f4fd47ed2d89f119c2f8b Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Tue, 5 May 2026 20:40:09 +0200 Subject: [PATCH 11/13] refactor: resolve object_id_field to concrete field at registration --- reversion/models.py | 8 ++++---- reversion/revisions.py | 19 ++++++++++--------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/reversion/models.py b/reversion/models.py index ee8d7e75..40e4c02a 100644 --- a/reversion/models.py +++ b/reversion/models.py @@ -19,8 +19,8 @@ from django.utils.translation import gettext_lazy as _ from reversion.errors import RevertError -from reversion.revisions import (_follow_relations_recursive, - _get_content_type, _get_options, _get_object_id_field) +from reversion.revisions import (_follow_relations_recursive, _get_content_type, + _get_options) logger = logging.getLogger(__name__) @@ -91,7 +91,7 @@ def revert(self, delete=False): model = version._model try: # Load the model instance from the same DB as it was saved under. - id_field = _get_object_id_field(model) + id_field = _get_options(model).object_id_field old_revision.add( model._default_manager.using(version.db).get(**{id_field: version.object_id}) ) @@ -146,7 +146,7 @@ def get_for_object(self, obj, model_db=None): def get_deleted(self, model, model_db=None): model_db = model_db or router.db_for_write(model) connection = connections[self.db] - object_id_field_name = _get_object_id_field(model) + object_id_field_name = _get_options(model).object_id_field if self.db == model_db and connection.vendor in ("sqlite", "postgresql", "oracle"): object_id_cast_target = model._meta.get_field(object_id_field_name) if django.VERSION >= (2, 1): diff --git a/reversion/revisions.py b/reversion/revisions.py index 1060c010..1420d26d 100644 --- a/reversion/revisions.py +++ b/reversion/revisions.py @@ -212,11 +212,6 @@ def add_to_revision(obj, model_db=None): _add_to_revision(obj, db, model_db, True) -def _get_object_id_field(model): - field = _get_options(model).object_id_field - return model._meta.pk.name if field == "pk" else field - - def _save_revision(versions, user=None, comment="", meta=(), date_created=None, using=None): from reversion.models import Revision from reversion.models import Version @@ -230,8 +225,8 @@ def _save_revision(versions, user=None, comment="", meta=(), date_created=None, db: frozenset(map( force_str, model._base_manager.using(db).filter( - **{f"{_get_object_id_field(model)}__in": pks} - ).values_list(_get_object_id_field(model), flat=True), + **{f"{_get_options(model).object_id_field}__in": pks} + ).values_list(_get_options(model).object_id_field, flat=True), )) for db, pks in db_pks.items() } @@ -386,7 +381,7 @@ def _get_senders_and_signals(model): def register(model=None, fields=None, exclude=(), follow=(), format="json", - for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False, object_id_field="pk"): + for_concrete_model=True, ignore_duplicates=False, use_natural_foreign_keys=False, object_id_field=None): def register(model): # Prevent multiple registration. if is_registered(model): @@ -395,6 +390,12 @@ def register(model): )) # Parse fields. opts = model._meta.concrete_model._meta + if object_id_field is None: + id_field = model._meta.pk.attname + else: + model._meta.get_field(object_id_field) + id_field = object_id_field + version_options = _VersionOptions( fields=tuple( field_name @@ -411,7 +412,7 @@ def register(model): for_concrete_model=for_concrete_model, ignore_duplicates=ignore_duplicates, use_natural_foreign_keys=use_natural_foreign_keys, - object_id_field=object_id_field, + object_id_field=id_field, ) # Register the model. _registered_models[_get_registration_key(model)] = version_options From d27560a7d61823a33d1f68a399001272d6ff3f5a Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Tue, 5 May 2026 20:40:12 +0200 Subject: [PATCH 12/13] perf: skip db query when object_id_field is pk in admin views --- reversion/admin.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/reversion/admin.py b/reversion/admin.py index 269ad770..a6c3b7bb 100644 --- a/reversion/admin.py +++ b/reversion/admin.py @@ -200,12 +200,17 @@ def _reversion_revisionform_view(self, request, version, template_name, extra_co version.revision.revert(delete=True) # Run the normal changeform view. with self.create_revision(request): - obj = get_object_or_404( - version._model._default_manager.using(version.db), - **{_get_options(version._model).object_id_field: version.object_id}, - ) + opts = _get_options(version._model) + if opts.object_id_field == version._model._meta.pk.attname: + obj_pk = version.object_id + else: + obj = get_object_or_404( + version._model._default_manager.using(version.db), + **{opts.object_id_field: version.object_id}, + ) + obj_pk = str(obj.pk) response = self.changeform_view( - request, quote(str(obj.pk)), request.path, extra_context + request, quote(obj_pk), request.path, extra_context ) # Decide on whether the keep the changes. if request.method == "POST" and response.status_code == 302: @@ -311,8 +316,12 @@ def history_view(self, request, object_id, extra_context=None): if not self.has_change_permission(request): raise PermissionDenied - obj = get_object_or_404(self.model, pk=unquote(object_id)) - reversion_object_id = str(getattr(obj, _get_options(self.model).object_id_field)) + version_opts = _get_options(self.model) + if version_opts.object_id_field == self.model._meta.pk.attname: + reversion_object_id = unquote(object_id) + else: + obj = get_object_or_404(self.model, pk=unquote(object_id)) + reversion_object_id = str(getattr(obj, version_opts.object_id_field)) opts = self.model._meta action_list = [ { From 7d81001e8e478bf963e25d4226aa603afcb59614 Mon Sep 17 00:00:00 2001 From: Philippe Ducasse Date: Tue, 5 May 2026 20:40:15 +0200 Subject: [PATCH 13/13] docs: document object_id_field parameter in register() --- docs/api.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/api.rst b/docs/api.rst index 29d153dd..40c7fbc6 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -187,6 +187,11 @@ Registration API See `Serialization of natural keys `_ + ``object_id_field=None`` + The name of the model field to use as the version's ``object_id``. Defaults to the model's primary key field. Use this when you want versions to be keyed on a different unique field, such as a slug. + + The field must exist on the model (validated at registration time). ``"pk"`` is not a valid value — use the concrete field name instead (e.g. ``"id"``). + .. Hint:: By default, django-reversion will not register any parent classes of a model that uses multi-table inheritance. If you wish to also add parent models to your revision, you must explicitly add their ``parent_ptr`` fields to the ``follow`` parameter when you register the model.