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

UnboundLocalError ('version') when creating deleted list in get_many_to_something() #41

Closed
frwickst opened this issue Jun 18, 2015 · 8 comments

Comments

@frwickst
Copy link

This is a bug that was introduced by theodore.therone@gmail.com 9c81a09

The error is:
local variable 'version' referenced before assignment

The variable version is used before it is assigned anything leading to the error.

@jedie
Copy link
Owner

jedie commented Jul 22, 2015

v0.5.4 release is out. Has this been fixed?

@frwickst
Copy link
Author

Nope, the problem still exists and keeps me from comparing my custom user model. While the OneToOne fix fixes the first parts of my problem, this problem still exist. The code that got merges from theodore should probably be reverted or fixed as this is a bug.

@jedie
Copy link
Owner

jedie commented Jul 23, 2015

Thanks for the feedback. Then i need a way to reproduce this bug and write a unitests for this case. Seems that you have the configuration for it.

Can you copy&paste a simple example code and a full traceback ?

@frwickst
Copy link
Author

Sure, if you need some code to test with I might have the time to create an example tomorrow. But if you read the code you can clearly see that version will not be assigned when the code is trying to use it on line 180 in admin.py. There is just no way for that variable being assigned when used there.

Stack trace:

File ".../lib/python2.7/site-packages/django/core/handlers/base.py" in get_response
  111.                     response = wrapped_callback(request, *callback_args, **callback_kwargs)
File ".../lib/python2.7/site-packages/django/utils/decorators.py" in _wrapped_view
  105.                     response = view_func(request, *args, **kwargs)
File ".../lib/python2.7/site-packages/django/views/decorators/cache.py" in _wrapped_view_func
  52.         response = view_func(request, *args, **kwargs)
File ".../lib/python2.7/site-packages/django/contrib/admin/sites.py" in inner
  206.             return view(request, *args, **kwargs)
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in compare_view
  719.         compare_data, has_unfollowed_fields = self.compare(obj, version1, version2)
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in compare
  674.             if not obj_compare.changed():
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in changed
  268.             info = self.get_m2o_change_info()
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in get_m2o_change_info
  311.         m2o_data1, m2o_data2 = self.get_reverse_foreign_key()
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in get_reverse_foreign_key
  303.         return self._get_both_results("get_reverse_foreign_key")
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in _get_both_results
  287.         result1 = self._get_result(self.compare_obj1, func_name)
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in _get_result
  283.         result = func()
File ".../python2.7/site-packages/reversion_compare/admin.py" in get_reverse_foreign_key
  124.         return self.get_many_to_something(ids, related_model, is_reverse=True)
File ".../lib/python2.7/site-packages/reversion_compare/admin.py" in get_many_to_something
  180.                     if ver.revision.date_created < version.revision.date_created:

Exception Type: UnboundLocalError at /<app_retracted>/<model_retracted>/3/history/compare/
Exception Value: local variable 'version' referenced before assignment

jedie referenced this issue Jul 23, 2015
also correctly scopes deleted for followed fields
@jedie
Copy link
Owner

jedie commented Jul 23, 2015

Sure, if you need some code to test with I might have the time to create an example tomorrow.

This would be great. If you have the time: create a unittest for it ;)

But if you read the code you can clearly see that version will not be assigned when the code is trying to use it on line 180 in admin.py. There is just no way for that variable being assigned when used there.

Yes, i see.

EDIT: Maybe:

 - if ver.revision.date_created < version.revision.date_created:
 + if ver.revision.date_created < old_revision.date_created:

@jedie jedie closed this as completed in fe7d02f Jul 23, 2015
@jedie
Copy link
Owner

jedie commented Jul 23, 2015

I add some test prints. The code part are running in the unittests. But in this case "version" object exist! Think because of the usage before, here:

for version in versions:
version.object_id = int(version.object_id)

I renamed "version.revision" to "old_revision"... The unittest passed: https://travis-ci.org/jedie/django-reversion-compare/builds/72319025

But i can't really see if this is the right behaviour :(

TODO: Add a unittest with "if self.has_int_pk:" == False and check if the if ver.revision.date_created < old_revision.date_created: code part does really the right thing...

@frwickst Can you test with the new commits?

@jedie jedie reopened this Jul 23, 2015
@frwickst
Copy link
Author

It works! Thanks for the fix, now I can finally compare my users history 👍

jedie added a commit that referenced this issue Jul 24, 2015
@jedie jedie closed this as completed Jul 24, 2015
@jedie
Copy link
Owner

jedie commented Jul 24, 2015

It works! Thanks for the fix, now I can finally compare my users history 👍

Thanks for feedback. i released v0.5.5

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

2 participants