Skip to content

Commit

Permalink
fix issue #7, allow package name to be set.
Browse files Browse the repository at this point in the history
register() will place records under given app.
Meta app_label will place both model / records under given app.
  • Loading branch information
dnozay authored and Damien Nozay committed Apr 23, 2013
1 parent 1156921 commit eee880b
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 2 deletions.
3 changes: 2 additions & 1 deletion runtests.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
'django.contrib.sessions',
'django.contrib.admin',
'simple_history',
'simple_history.tests'
'simple_history.tests',
'simple_history.tests.external'
),
DATABASES={
'default': {
Expand Down
11 changes: 10 additions & 1 deletion simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ def create_history_model(self, model):
"""
attrs = {'__module__': self.module}

app_module = models.get_app(model._meta.app_label)

This comment has been minimized.

Copy link
@treyhunner

treyhunner Apr 24, 2013

Member

This line is causing import errors when running tests in my project that uses django-simple-history.

This is run when the model classes are being created so many Django imports cannot be reliably used. I'm not sure why this one is causing my project to fail, but we'll need to find a different way to do this.

This comment has been minimized.

Copy link
@dnozay

dnozay Apr 25, 2013

Author Contributor

experimenting with importlib is worth a try, maybe imp.find_module could do the trick. I don't know where you use it and if it is failing because of some weird import cycle so I can't say for sure.

This comment has been minimized.

Copy link
@mghantous

mghantous May 14, 2013

I am also experiencing problems with this. @treyhunner, are you receiving an "ImportError haystack: cannot import name [XYZ]" error when trying to perform a migration? Is there another issue open to track this?

This comment has been minimized.

Copy link
@treyhunner

treyhunner May 14, 2013

Member

@mghantous here is a project that demonstrates the error I'm receiving: https://github.com/treyhunner/simple-history-error-demo

I made issue #42 for this problem. Feel free to comment with more details there or give another example of the problem.

if model.__module__ != self.module:

This comment has been minimized.

Copy link
@treyhunner

treyhunner Apr 24, 2013

Member

Isn't the if clause in this if-else statement redundant? It looks like attrs['__module__'] would already be set to self.module by default at this point.

This comment has been minimized.

Copy link
@dnozay

dnozay Apr 25, 2013

Author Contributor

no, when executing this path from register(model, app="foobar") then model.__module__ will be different from "foobar" (most likely). This is the test used to check whether register was called with an app parameter.

# registered under different app
attrs['__module__'] = self.module
elif app_module.__name__ != self.module:
# has meta options with app_label
attrs['__module__'] = app_module.__name__

fields = self.copy_fields(model)
attrs.update(fields)
attrs.update(self.get_extra_fields(model, fields))
Expand Down Expand Up @@ -135,9 +143,10 @@ def get_meta_options(self, model):
Returns a dictionary of fields that will be added to
the Meta inner class of the historical record model.
"""
return {
options = {
'ordering': ('-history_date', '-history_id'),
}
return options

def post_save(self, instance, created, **kwargs):
if not created and hasattr(instance, 'skip_history_when_saving'):
Expand Down
Empty file.
14 changes: 14 additions & 0 deletions simple_history/tests/external/models.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

from django.db import models
from simple_history.models import HistoricalRecords
from simple_history import register

class ExternalModel2(models.Model):
name = models.CharField(max_length=100)
history = HistoricalRecords()

class ExternalModel4(models.Model):
name = models.CharField(max_length=100)

register(ExternalModel4, app='simple_history.tests',
manager_name='histories')
11 changes: 11 additions & 0 deletions simple_history/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,14 @@ def _history_user(self):
register(User, app='simple_history.tests', manager_name='histories')


class ExternalModel1(models.Model):
name = models.CharField(max_length=100)
history = HistoricalRecords()
class Meta:
app_label = 'external'

class ExternalModel3(models.Model):
name = models.CharField(max_length=100)

register(ExternalModel3, app='simple_history.tests.external',
manager_name='histories')
28 changes: 28 additions & 0 deletions simple_history/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
from django.contrib.auth.models import User

from .models import Poll, Choice, Restaurant, Person, FileModel, Document
from .models import ExternalModel1, ExternalModel3
from simple_history.tests.external.models import ExternalModel2, ExternalModel4

today = datetime(2021, 1, 1, 10, 0)
tomorrow = today + timedelta(days=1)
Expand Down Expand Up @@ -193,6 +195,32 @@ def test_register_separate_app(self):
self.assertEqual(len(User.histories.all()), 1)
self.assertEqual(len(user.histories.all()), 1)

class AppLabelTest(TestCase):
def get_table_name(self, manager):
return manager.model._meta.db_table

def test_explicit_app_label(self):
self.assertEqual(self.get_table_name(ExternalModel1.objects),
'external_externalmodel1')
self.assertEqual(self.get_table_name(ExternalModel1.history),
'external_historicalexternalmodel1')

def test_default_app_label(self):
self.assertEqual(self.get_table_name(ExternalModel2.objects),
'external_externalmodel2')
self.assertEqual(self.get_table_name(ExternalModel2.history),
'external_historicalexternalmodel2')

def test_register_app_label(self):
self.assertEqual(self.get_table_name(ExternalModel3.objects),
'tests_externalmodel3')
self.assertEqual(self.get_table_name(ExternalModel3.histories),
'external_historicalexternalmodel3')
self.assertEqual(self.get_table_name(ExternalModel4.objects),
'external_externalmodel4')
self.assertEqual(self.get_table_name(ExternalModel4.histories),
'tests_historicalexternalmodel4')


class HistoryManagerTest(TestCase):
def test_most_recent(self):
Expand Down

0 comments on commit eee880b

Please sign in to comment.