Skip to content

Commit 52d0648

Browse files
rwlogelRoss Mechanic
authored andcommitted
Correct module for history model of inherited model
If base model is in a different module the history model for the inherited model should be in the same module as the inherited model
1 parent 29da7a5 commit 52d0648

File tree

4 files changed

+40
-8
lines changed

4 files changed

+40
-8
lines changed

simple_history/models.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,13 +73,15 @@ def save_without_historical_record(self, *args, **kwargs):
7373
save_without_historical_record)
7474

7575
def finalize(self, sender, **kwargs):
76+
inherited = False
7677
try:
7778
hint_class = self.cls
7879
except AttributeError: # called via `register`
7980
pass
8081
else:
8182
if hint_class is not sender: # set in concrete
82-
if not (self.inherit and issubclass(sender, hint_class)):
83+
inherited = (self.inherit and issubclass(sender, hint_class))
84+
if not inherited:
8385
return # set in abstract
8486
if hasattr(sender._meta, 'simple_history_manager_attribute'):
8587
raise exceptions.MultipleRegistrationsError(
@@ -88,8 +90,12 @@ def finalize(self, sender, **kwargs):
8890
sender._meta.object_name,
8991
)
9092
)
91-
history_model = self.create_history_model(sender)
92-
module = importlib.import_module(self.module)
93+
history_model = self.create_history_model(sender, inherited)
94+
if inherited:
95+
# Make sure history model is in same module as inherited class
96+
module = importlib.import_module(history_model.__module__)
97+
else:
98+
module = importlib.import_module(self.module)
9399
setattr(module, history_model.__name__, history_model)
94100

95101
# The HistoricalRecords object will be discarded,
@@ -103,14 +109,18 @@ def finalize(self, sender, **kwargs):
103109
setattr(sender, self.manager_name, descriptor)
104110
sender._meta.simple_history_manager_attribute = self.manager_name
105111

106-
def create_history_model(self, model):
112+
def create_history_model(self, model, inherited):
107113
"""
108114
Creates a historical model to associate with the model provided.
109115
"""
110116
attrs = {'__module__': self.module}
111117

112118
app_module = '%s.models' % model._meta.app_label
113-
if model.__module__ != self.module:
119+
120+
if inherited:
121+
# inherited use models module
122+
attrs['__module__'] = model.__module__
123+
elif model.__module__ != self.module:
114124
# registered under different app
115125
attrs['__module__'] = self.module
116126
elif app_module != self.module:

simple_history/registry_tests/tests.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,21 @@ def test_accessor_override(self):
7171
assert hasattr(User, 'my_history_model_accessor')
7272

7373

74+
class TestInheritedModule(TestCase):
75+
76+
def test_using_app_label(self):
77+
try:
78+
from ..tests.models import HistoricalConcreteExternal
79+
except ImportError:
80+
self.fail("HistoricalConcreteExternal is in wrong module")
81+
82+
def test_default(self):
83+
try:
84+
from ..tests.models import HistoricalConcreteExternal2
85+
except ImportError:
86+
self.fail("HistoricalConcreteExternal2 is in wrong module")
87+
88+
7489
class TestTrackingInheritance(TestCase):
7590

7691
def test_tracked_abstract_base(self):

simple_history/tests/models.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,13 @@ class Meta:
355355
app_label = 'tests'
356356

357357

358+
class ConcreteExternal2(AbstractExternal):
359+
name = models.CharField(max_length=50)
360+
361+
class Meta:
362+
pass # Don't set app_label to test inherited module path
363+
364+
358365
class TrackedWithAbstractBase(TrackedAbstractBaseA):
359366
pass
360367

simple_history/tests/tests/test_models.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ def test_create_history_model_with_one_to_one_field_to_integer_field(self):
345345
records = HistoricalRecords()
346346
records.module = AdminProfile.__module__
347347
try:
348-
records.create_history_model(AdminProfile)
348+
records.create_history_model(AdminProfile, False)
349349
except:
350350
self.fail("SimpleHistory should handle foreign keys to one to one"
351351
"fields to integer fields without throwing an exception")
@@ -354,7 +354,7 @@ def test_create_history_model_with_one_to_one_field_to_char_field(self):
354354
records = HistoricalRecords()
355355
records.module = Bookcase.__module__
356356
try:
357-
records.create_history_model(Bookcase)
357+
records.create_history_model(Bookcase, False)
358358
except:
359359
self.fail("SimpleHistory should handle foreign keys to one to one"
360360
"fields to char fields without throwing an exception.")
@@ -363,7 +363,7 @@ def test_create_history_model_with_multiple_one_to_ones(self):
363363
records = HistoricalRecords()
364364
records.module = MultiOneToOne.__module__
365365
try:
366-
records.create_history_model(MultiOneToOne)
366+
records.create_history_model(MultiOneToOne, False)
367367
except:
368368
self.fail("SimpleHistory should handle foreign keys to one to one"
369369
"fields to one to one fields without throwing an "

0 commit comments

Comments
 (0)