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

ForeignKey relation #67

Open
DiogoMarques29 opened this issue Apr 20, 2016 · 5 comments
Open

ForeignKey relation #67

DiogoMarques29 opened this issue Apr 20, 2016 · 5 comments

Comments

@DiogoMarques29
Copy link

Hi,

I was trying out your app and noticed something weird. Why is a normal ForeignKey (that we are not registering in reversion) showing up on the diff panel? Even if it's empty it always shows 'add: None'. That does not make any sense since this class has the information about that ForeignKey (throw a relational ID). Then I started watching the code and, if I saw correctly, these fields are treated as reversed relations, which they are not.

@amureki
Copy link
Contributor

amureki commented May 7, 2016

@DiogoMarques29 have same issue here
I guess, problem is here:
https://github.com/jedie/django-reversion-compare/blob/master/reversion_compare/compare.py#L86

Not sure, how better to compare fields, seems like comparing values is not good enough?

@jedie
Copy link
Owner

jedie commented Aug 26, 2016

Any idea to fix this? Pull requests are welcome ;)

@alaruss
Copy link

alaruss commented Mar 27, 2017

Hello,

I fixed this by changing https://github.com/jedie/django-reversion-compare/blob/master/reversion_compare/compare.py#L46 to:
self.value = version_record.field_dict.get(getattr(field, 'attname', field_name), DOES_NOT_EXIST)

And removing this if statement: https://github.com/jedie/django-reversion-compare/blob/master/reversion_compare/compare.py#L92

Frankly, I can't get why in case of the ForeignKey the whole object should be compared.

@jedie
Copy link
Owner

jedie commented Oct 2, 2017

@alaruss can you contribute a pull request with tests?!?

@alaruss
Copy link

alaruss commented Oct 5, 2017

@jedie Something like this #100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants