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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃専 Add support for Django2.0 #121

Merged
merged 3 commits into from
Jul 26, 2018
Merged

馃専 Add support for Django2.0 #121

merged 3 commits into from
Jul 26, 2018

Conversation

dodobas
Copy link
Contributor

@dodobas dodobas commented Jul 16, 2018

No description provided.

@dodobas dodobas changed the title WIP: Add support for Django2.0 馃専 Add support for Django2.0 Jul 16, 2018
@@ -28,7 +28,7 @@ def process_response(self, request, response):


class ProfileMiddleware(MiddlewareMixin):
def __init__(self):
def __init__(self, get_response=None):
pass

def process_view(self, request, view, *args, **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

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

This middleware should be updated

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind

@@ -28,7 +28,7 @@ def process_response(self, request, response):


class ProfileMiddleware(MiddlewareMixin):
Copy link
Contributor

Choose a reason for hiding this comment

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

MiddlewareMixin is no longer needed

.travis.yml Outdated
- DJANGO_VERSION=1.11.2
- DJANGO_VERSION=1.10.8
- DJANGO_VERSION=1.11.14
- DJANGO_VERSION=2.0.7
Copy link
Member

Choose a reason for hiding this comment

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

Think we can drop support for Python 2.7 and Django 1.10

README.md Outdated
@@ -22,7 +22,7 @@ Smartmin tries to stay in lock step with the latest Django versions. With each n
will be released and we will save the major changes (possibly breaking backwards compatibility) on these versions. This
includes updating to the latest version of Twitter Bootstrap.

The latest version is the 1.11.* series which works against Django 1.9, 1.10 and 1.11.
The latest version is the 2.0.* series which works against Django 1.10, 1.11 and 2.0.
Copy link
Member

Choose a reason for hiding this comment

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

as above

@@ -904,7 +904,7 @@ def test_lockout(self):
# try to log in four times
for i in range(4):
response = self.client.post(login_url, post_data)
self.assertFalse(response.context['user'].is_authenticated())
self.assertFalse(response.context['user'].is_authenticated)
Copy link
Member

Choose a reason for hiding this comment

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

so this was a method in Django 1.x but a field in Django 2.x? Problem with this test is that if is_authenticated is a method.. then is always true.. I don't get how this is working...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assertFalse is a simple truthy check if cond: ... and a is_authenticated method is not 'undefined' so it's always 'true'

Copy link
Member

Choose a reason for hiding this comment

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

Ok I just poked around the Django src and it seems is_authenticated has been a property for a while - but it returns a magical CallableFalse which can also be called. Makes sense now.

@dodobas dodobas force-pushed the support_django2 branch 2 times, most recently from ea236e3 to b5ec057 Compare July 25, 2018 11:00
@dodobas
Copy link
Contributor Author

dodobas commented Jul 25, 2018

Closes #120

@rowanseymour rowanseymour merged commit fd99c8a into master Jul 26, 2018
@rowanseymour rowanseymour deleted the support_django2 branch July 26, 2018 13:27
@rowanseymour rowanseymour mentioned this pull request Jul 26, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants