From f54c5f6a9cf58cb5913cccdd665fb02ef7274cc9 Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 14 Jan 2019 17:47:50 -0500 Subject: [PATCH 1/5] GH-512: Fix bug that disallowed using 'self' str when creating ForeignKey to self --- simple_history/models.py | 7 +++++++ simple_history/tests/models.py | 6 ++++++ simple_history/tests/tests/test_models.py | 22 +++++++++++++++++++--- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/simple_history/models.py b/simple_history/models.py index 96f1c3108..fabf13878 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -213,6 +213,13 @@ def copy_fields(self, model): else: FieldType = type(old_field) + # If field_args['to'] is 'self' then we have a case where the object + # has a foreign key to itself. If we pass the historical record's + # field to = 'self', the foreign key will point to an historical + # record rather than the base record. We can use old_field.model here. + if field_args.get('to', None) == 'self': + field_args['to'] = old_field.model + # Override certain arguments passed when creating the field # so that they work for the historical field. field_args.update( diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index 1b91a1097..d5793b342 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -553,3 +553,9 @@ class CustomNameModel(models.Model): class CustomManagerNameModel(models.Model): name = models.CharField(max_length=15) log = HistoricalRecords() + + +class ForeignKeyToSelfModel(models.Model): + fk_to_self = models.ForeignKey("ForeignKeyToSelfModel", null=True, related_name="+") + fk_to_self_using_str = models.ForeignKey("self", null=True, related_name="+") + history = HistoricalRecords() diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 4fc74d13e..0c78c6656 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import unittest + import uuid import warnings from datetime import datetime, timedelta @@ -43,6 +44,7 @@ ExternalModel1, ExternalModel3, FileModel, + ForeignKeyToSelfModel, HistoricalChoice, HistoricalCustomFKError, HistoricalPoll, @@ -583,7 +585,6 @@ def setUp(self): self.poll.save() def test_get_prev_record(self): - self.poll.question = "ask questions?" self.poll.save() self.poll.question = "eh?" @@ -1203,7 +1204,6 @@ def test_signal_is_able_to_retrieve_request_from_thread(self): class WarningOnAbstractModelWithInheritFalseTest(TestCase): def test_warning_on_abstract_model_with_inherit_false(self): - with warnings.catch_warnings(record=True) as w: class AbstractModelWithInheritFalse(models.Model): @@ -1224,7 +1224,6 @@ class Meta: class MultiDBWithUsingTest(TestCase): - """Asserts historical manager respects `using()` and the `using` keyword argument in `save()`. """ @@ -1309,3 +1308,20 @@ def test_multidb_with_using_keyword_in_save_and_delete(self): .order_by("history_date") ], ) + + +class ForeignKeyToSelfTest(TestCase): + def setUp(self): + self.model = ForeignKeyToSelfModel + self.history_model = self.model.history.model + + def test_foreign_key_to_self_using_model_str(self): + self.assertEqual( + self.model, self.history_model.fk_to_self.field.remote_field.model + ) + + def test_foreign_key_to_self_using_self_str(self): + self.assertEqual( + ForeignKeyToSelfModel, + self.history_model.fk_to_self_using_str.field.remote_field.model, + ) From 8b741c8e960c284a39ebaee3445a50c82eaf29e3 Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 14 Jan 2019 17:56:52 -0500 Subject: [PATCH 2/5] Formatted --- simple_history/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/simple_history/models.py b/simple_history/models.py index fabf13878..49f13ddb3 100644 --- a/simple_history/models.py +++ b/simple_history/models.py @@ -217,8 +217,8 @@ def copy_fields(self, model): # has a foreign key to itself. If we pass the historical record's # field to = 'self', the foreign key will point to an historical # record rather than the base record. We can use old_field.model here. - if field_args.get('to', None) == 'self': - field_args['to'] = old_field.model + if field_args.get("to", None) == "self": + field_args["to"] = old_field.model # Override certain arguments passed when creating the field # so that they work for the historical field. From 5be8ce4a4196c0b035a105a7d44c8fc045543ffc Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 14 Jan 2019 18:00:27 -0500 Subject: [PATCH 3/5] Updated CHANGES.rst --- CHANGES.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.rst b/CHANGES.rst index bc1caf2c5..e1ff9470f 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,7 @@ Unreleased - Add support for `using` chained manager method and save/delete keyword argument (gh-507) - Added management command `clean_duplicate_history` to remove duplicate history entries (gh-483) - Updated most_recent to work with excluded_fields (gh-477) +- Fixed bug that prevented self-referential foreign key from using `'self'` (gh-513) 2.6.0 (2018-12-12) ------------------ From eb3f0842445c0e29435154b0edb167750f83fe95 Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 14 Jan 2019 18:02:39 -0500 Subject: [PATCH 4/5] last change --- simple_history/tests/tests/test_models.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/simple_history/tests/tests/test_models.py b/simple_history/tests/tests/test_models.py index 0c78c6656..531dd16da 100644 --- a/simple_history/tests/tests/test_models.py +++ b/simple_history/tests/tests/test_models.py @@ -1322,6 +1322,5 @@ def test_foreign_key_to_self_using_model_str(self): def test_foreign_key_to_self_using_self_str(self): self.assertEqual( - ForeignKeyToSelfModel, - self.history_model.fk_to_self_using_str.field.remote_field.model, + self.model, self.history_model.fk_to_self_using_str.field.remote_field.model ) From db5b140b044b70cc4f4f7946b5bad0deba652fd4 Mon Sep 17 00:00:00 2001 From: Ross Mechanic Date: Mon, 14 Jan 2019 18:10:49 -0500 Subject: [PATCH 5/5] Fixed missing on_delete --- simple_history/tests/models.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/simple_history/tests/models.py b/simple_history/tests/models.py index d5793b342..f1465fdf4 100644 --- a/simple_history/tests/models.py +++ b/simple_history/tests/models.py @@ -556,6 +556,10 @@ class CustomManagerNameModel(models.Model): class ForeignKeyToSelfModel(models.Model): - fk_to_self = models.ForeignKey("ForeignKeyToSelfModel", null=True, related_name="+") - fk_to_self_using_str = models.ForeignKey("self", null=True, related_name="+") + fk_to_self = models.ForeignKey( + "ForeignKeyToSelfModel", null=True, related_name="+", on_delete=models.CASCADE + ) + fk_to_self_using_str = models.ForeignKey( + "self", null=True, related_name="+", on_delete=models.CASCADE + ) history = HistoricalRecords()