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.

Sign up for GitHub

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

Prepare for Django 2.0 #320

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7cc283e
Update models.py for Django 2.0
flesser Oct 20, 2017
32f2dc2
Update admin.py for Django 2.0
flesser Oct 20, 2017
0d73d84
Swap imports
flesser Oct 20, 2017
48147a5
Updated `is_authenticated` to be a property.
dgilge Nov 28, 2017
e36a105
Merge pull request #1 from dgilge/django2.0
flesser Nov 28, 2017
38c72ca
Improved implementation for is_authenticated
dgilge Nov 28, 2017
41c5965
is_authenticated shouldn't emit a warning in Django 1.10+ anymore.
dgilge Dec 4, 2017
212dd35
Added on_delete to some test model fields
dgilge Dec 4, 2017
49d53d4
Swap import in test
dgilge Dec 4, 2017
07b206c
Updated urls in tests
dgilge Dec 4, 2017
b5a32b9
Merge pull request #2 from dgilge/django2.0
flesser Dec 4, 2017
5622e85
Merge fix for Django 2.0 with new self-referential model logic
Dec 20, 2017
701a815
Merge pull request #3 from RossRogers/django2.0
flesser Dec 21, 2017
bd58b70
Correct @flesser's merge to remove new uses of `old_field.rel.to`. R…
Dec 21, 2017
a75adab
Merge pull request #4 from RossRogers/django2.0
flesser Dec 22, 2017
2fe4038
Clean up changes for Django 2.0, update tests
macro1 Mar 18, 2018
c8e09e8
Update AUTHORS and CHANGES
macro1 Mar 18, 2018
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
5 changes: 4 additions & 1 deletion simple_history/admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,17 @@
from django.contrib import admin
from django.contrib.admin import helpers
from django.contrib.contenttypes.models import ContentType
from django.core.urlresolvers import reverse
from django.shortcuts import get_object_or_404, render
from django.utils.text import capfirst
from django.utils.html import mark_safe
from django.utils.translation import ugettext as _
from django.utils.encoding import force_text
from django.conf import settings

try:
from django.urls import reverse
except ImportError: # Django < 1.10
from django.core.urlresolvers import reverse
try:
from django.contrib.admin.utils import unquote
except ImportError: # Django < 1.7
Expand Down
11 changes: 9 additions & 2 deletions simple_history/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ def copy_fields(self, model):
if getattr(old_field, 'db_column', None):
field_arguments['db_column'] = old_field.db_column
field = FieldType(
old_field.rel.to,
# required for Django <= 1.8 # required for Django >= 2.0
old_field.rel.to if hasattr(old_field, 'rel') else old_field.remote_field.model,
Copy link

Choose a reason for hiding this comment

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

Having this inline makes for a dense line IMHO.

Copy link

Choose a reason for hiding this comment

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

Suggest, for readability

if hasattr(old_field, 'rel'):
    target = old_field.rel.to  # Django <= 1.8 
else:
    target = old_field.remote_field.model # Django >= 2.0
field = FieldType(target,
    ...

Copy link

Choose a reason for hiding this comment

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

This could even be determined just once and saved (in this module before this class, or in a shared compat module).

Copy link

Choose a reason for hiding this comment

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

Also I think the test should be for the new attribute name; both exists in django 1.11 but the old-style one causes a warning.

related_name='+',
null=True,
blank=True,
Expand Down Expand Up @@ -275,7 +276,13 @@ def get_history_user(self, instance):
return instance._history_user
except AttributeError:
try:
if self.thread.request.user.is_authenticated():
try:
# Django < 1.10
is_authenticated = self.thread.request.user.is_authenticated()
Copy link

Choose a reason for hiding this comment

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

This will work but emit warnings in 1.10+

except TypeError:
# Django >= 2.0
is_authenticated = self.thread.request.user.is_authenticated
if is_authenticated:
return self.thread.request.user
return None
except AttributeError:
Expand Down