Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Foreign keys which aren't integers #38

Closed
jfyne opened this issue May 7, 2013 · 6 comments
Closed

Foreign keys which aren't integers #38

jfyne opened this issue May 7, 2013 · 6 comments

Comments

@jfyne
Copy link
Contributor

jfyne commented May 7, 2013

If I have a model which defines a custom primary key like this:

iso_code = models.CharField(max_length=3, primary_key=True)

When the model is referred to as a foreign key saving fails as the history table automatically uses integer fields for ForeignKey fields. I attempted a fix but I am having some issues with it. What I tried was this

https://github.com/essencedigital/django-simple-history/commit/ccf363c4209e53d308ee408e0bdf98900b58bd77

But it doesn't work with what appears to be a foreign key that looks like this

last_issued_version = models.ForeignKey('io.InsertionOrderVersion', null=True, blank=True, related_name='insertionorder_last_issued_version')

Any ideas?

Thanks

@treyhunner
Copy link
Member

Thanks for reporting the issue. The assumptions django-simple-history has made about foreign keys are too narrow (see #13 also). I definitely want to fix the foreign key and primary key issues. I haven't done it myself yet because I haven't had any custom primary/foreign keys in my own projects so far.

You might want to check whether João Pedro Francese's fork on Bitbucket has a solution for either of these issues. If it does then we should probably figure out what change fixes the issue and incorporate it into this version. There are a lot of little differences between these two versions of django-simple-history and I haven't yet cherry-picked all the useful changes into this one.

A failing test for this would also be greatly appreciated.

@dnozay
Copy link
Contributor

dnozay commented May 7, 2013

field.null = True
field.blank = True
field.related = None
field.related_name = None
fk = True
...
# this part may need to change
if fk:
    field.name = field.name + "_id"

@jfyne
Copy link
Contributor Author

jfyne commented May 7, 2013

@dnozay
Copy link
Contributor

dnozay commented May 7, 2013

see pull request #41

@thenewguy
Copy link

I do not know how much has changed, but I dealt with this before. https://bitbucket.org/pendletongp/django-simple-history

If I remember correctly, I added support for non-integer primary keys and relation fields.

It has been a long time since I worked with this but maybe it will be helpful. It looks like I added some test cases too.

treyhunner added a commit that referenced this issue May 10, 2013
treyhunner added a commit that referenced this issue May 25, 2013
add tests + foreign-key fix (issue #38)
@dnozay
Copy link
Contributor

dnozay commented May 25, 2013

fixed in pull request #41 and commit 68a91b1

@dnozay dnozay closed this as completed May 25, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants