Skip to content

Commit

Permalink
Merge pull request #55 from daniell/master
Browse files Browse the repository at this point in the history
Fixes bug with foreign keys to one-to-one fields
  • Loading branch information
treyhunner committed Jul 28, 2013
2 parents 0f2d604 + 61613af commit 9142aeb
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,6 @@ MANIFEST
.tox/
htmlcov/
test_files/
/.ve
/.project
/.pydevproject
1 change: 1 addition & 0 deletions AUTHORS.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Authors
- Aleksey Kladov
- Corey Bertram
- Damien Nozay
- Daniel Levy
- George Vilches
- Hamish Downer
- Joao Pedro Francese
Expand Down
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ Changes

tip (unreleased)
----------------
- Fixed error that occurs when models have a foreign key pointing to a one to one field.

- Allow non-integer foreign keys
- Allow foreign keys referencing the name of the model as a string
Expand Down
23 changes: 20 additions & 3 deletions simple_history/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,25 @@ class CustomForeignKey(parent_type):

def get_attname(self):
return self.name

def do_related_class(self, other, cls):
def get_field(self, other, cls):
# this hooks into contribute_to_class() and this is
# called specifically after the class_prepared signal
to_field = copy.copy(self.rel.to._meta.pk)
field = self
if isinstance(to_field, models.AutoField):
if isinstance(to_field, models.OneToOneField):
#HACK This creates a new custom foreign key based on to_field,
# and calls itself with that, effectively making the calls
# recursive
temp_field = self.__class__(to_field.rel.to._meta.object_name)
for key, val in to_field.__dict__.items():
if (isinstance(key, basestring)
and not key.startswith('_')):
setattr(temp_field, key, val)
field = self.__class__.get_field(
temp_field, other, to_field.rel.to)

elif isinstance(to_field, models.AutoField):
field.__class__ = models.IntegerField
else:
field.__class__ = to_field.__class__
Expand Down Expand Up @@ -217,6 +229,11 @@ def do_related_class(self, other, cls):
and not key.startswith(excluded_prefixes)
and not key in excluded_attributes):
setattr(field, key, val)
return field

def do_related_class(self, other, cls):
field = self.get_field(other, cls)

transform_field(field)
field.rel = None

Expand Down
21 changes: 21 additions & 0 deletions simple_history/tests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,12 @@ class Document(models.Model):
def _history_user(self):
return self.changed_by

class Profile(User):
date_of_birth = models.DateField()

class AdminProfile(models.Model):
profile = models.ForeignKey(Profile)

class State(models.Model):
library = models.ForeignKey('Library', null=True)
history = HistoricalRecords()
Expand All @@ -71,11 +76,27 @@ class Book(models.Model):
isbn = models.CharField(max_length=15, primary_key=True)
history = HistoricalRecords()

class HardbackBook(Book):
price = models.FloatField()

class Bookcase(models.Model):
books = models.ForeignKey(HardbackBook)

class Library(models.Model):
book = models.ForeignKey(Book, null=True)
history = HistoricalRecords()

class BaseModel(models.Model):
pass

class FirstLevelInheritedModel(BaseModel):
pass

class SecondLevelInheritedModel(FirstLevelInheritedModel):
pass

class MultiOneToOne(models.Model):
fk = models.ForeignKey(SecondLevelInheritedModel)

class SelfFK(models.Model):
fk = models.ForeignKey('self', null=True)
Expand Down
34 changes: 33 additions & 1 deletion simple_history/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
from django_webtest import WebTest
from django.core.files.base import ContentFile
from django.core.urlresolvers import reverse
from simple_history.tests.models import Profile, AdminProfile, Bookcase,\
MultiOneToOne
from django.db import models
from simple_history.models import HistoricalRecords
try:
from django.contrib.auth import get_user_model
User = get_user_model()
Expand Down Expand Up @@ -226,7 +230,6 @@ def test_raw_save(self):
'history_type': "~",
})


class RegisterTest(TestCase):
def test_register_no_args(self):
self.assertEqual(len(Choice.history.all()), 0)
Expand All @@ -250,6 +253,35 @@ def test_reregister(self):
self.assertTrue(hasattr(User, 'histories'))
self.assertFalse(hasattr(User, 'again'))

class CreateHistoryModelTests(TestCase):

def test_create_history_model_with_one_to_one_field_to_integer_field(self):
records = HistoricalRecords()
records.module = AdminProfile.__module__
try:
records.create_history_model(AdminProfile)
except:
self.fail("SimpleHistory should handle foreign keys to one to one"
"fields to integer fields without throwing an exception.")

def test_create_history_model_with_one_to_one_field_to_char_field(self):
records = HistoricalRecords()
records.module = Bookcase.__module__
try:
records.create_history_model(Bookcase)
except:
self.fail("SimpleHistory should handle foreign keys to one to one"
"fields to char fields without throwing an exception.")

def test_create_history_model_with_multiple_one_to_ones(self):
records = HistoricalRecords()
records.module = MultiOneToOne.__module__
try:
records.create_history_model(MultiOneToOne)
except:
self.fail("SimpleHistory should handle foreign keys to one to one"
"fields to one to one fields without throwing an "
"exception.")

class AppLabelTest(TestCase):
def get_table_name(self, manager):
Expand Down

0 comments on commit 9142aeb

Please sign in to comment.