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

Prepare for Django 2.0 #320

Closed
wants to merge 17 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
is_authenticated shouldn't emit a warning in Django 1.10+ anymore.
Note: In Django 2.0 the AttributeError is raised in all passing tests right now.
dgilge committed Dec 4, 2017
commit 41c5965f69b9faba858c34911b41b6e6d0c611dc
12 changes: 5 additions & 7 deletions simple_history/models.py
Original file line number Diff line number Diff line change
@@ -277,15 +277,13 @@ def get_history_user(self, instance):
except AttributeError:
try:
is_authenticated = self.thread.request.user.is_authenticated
try:
is_authenticated = is_authenticated() # Django < 1.10
except TypeError:
pass
if is_authenticated:
return self.thread.request.user
return None
except AttributeError:
return None
if not is_authenticated in (True, False):
is_authenticated = is_authenticated() # Django < 1.10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not correct. In 1.10 or 1.11, is_authenticated is an instance of CallableTrue or CallableFalse, not True or False, so the if not in test will fail and go into line 283 which will raise the deprecation warning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that might be. I tested it having the callable boolean in mind but now I saw that none of the implemented tests get to this line at all in Django 1.10+ (because self.thread.request.user.is_authenticated raises the caught AttributeError.

request.user raises an AttributeError in multiple tests in Django 2.0 (AttributeError: 'WSGIRequest' object has no attribute 'user'). For that reason it might be the next step to fix those.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error might be caused by a wrong ordering of middleware.

if is_authenticated:
return self.thread.request.user
return None


def transform_field(field):