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

Django 1.11 upgrade #4750

Merged
merged 13 commits into from
Oct 23, 2018
4 changes: 2 additions & 2 deletions readthedocs/api/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from django.core.cache import cache
from django.shortcuts import get_object_or_404
from tastypie import fields
from tastypie.authorization import DjangoAuthorization
from tastypie.authorization import DjangoAuthorization, ReadOnlyAuthorization
from tastypie.constants import ALL, ALL_WITH_RELATIONS
from tastypie.http import HttpCreated
from tastypie.resources import ModelResource
Expand All @@ -39,7 +39,7 @@ class Meta(object):
allowed_methods = ['get', 'post', 'put']
queryset = Project.objects.api()
authentication = PostAuthentication()
authorization = DjangoAuthorization()
authorization = ReadOnlyAuthorization()
excludes = ['path', 'featured', 'programming_language']
filtering = {
'users': ALL_WITH_RELATIONS,
Expand Down
6 changes: 3 additions & 3 deletions readthedocs/core/fixtures/eric.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"last_login": "2010-08-14 01:51:05",
"groups": [],
"user_permissions": [],
"password": "sha1$035cb$156ad6cb44332fb4f24bcb634142a67435be0b37",
"password": "pbkdf2_sha256$30000$Vs87OlKZEzCb$nUw1o5pGQw7ff/QhnleSpUOupBaT1DogZrVaoZyQRyc=",
"email": "e@e.co",
"date_joined": "2010-08-14 01:50:58"
}
Expand All @@ -30,7 +30,7 @@
"last_login": "2010-08-14 01:51:05",
"groups": [],
"user_permissions": [],
"password": "sha1$035cb$156ad6cb44332fb4f24bcb634142a67435be0b37",
"password": "pbkdf2_sha256$30000$Vs87OlKZEzCb$nUw1o5pGQw7ff/QhnleSpUOupBaT1DogZrVaoZyQRyc=",
"email": "e@etest.co",
"date_joined": "2010-08-14 01:50:58"
}
Expand All @@ -48,7 +48,7 @@
"last_login": "2010-08-14 01:51:05",
"groups": [],
"user_permissions": [],
"password": "sha1$035cb$156ad6cb44332fb4f24bcb634142a67435be0b37",
"password": "pbkdf2_sha256$30000$Vs87OlKZEzCb$nUw1o5pGQw7ff/QhnleSpUOupBaT1DogZrVaoZyQRyc=",
"email": "e@e.co",
"date_joined": "2010-08-14 01:50:58"
}
Expand Down
22 changes: 12 additions & 10 deletions readthedocs/core/middleware.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,21 @@
"""Middleware for core app."""

from __future__ import absolute_import
from builtins import object
from __future__ import (
absolute_import, division, print_function, unicode_literals)

import logging

from django.utils.translation import ugettext_lazy as _
from django.conf import settings
from django.contrib.sessions.middleware import SessionMiddleware
from django.core.cache import cache
from django.core.exceptions import ObjectDoesNotExist, MultipleObjectsReturned
from django.core.urlresolvers import set_urlconf, get_urlconf
from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist
from django.core.urlresolvers import get_urlconf, set_urlconf
from django.http import Http404, HttpResponseBadRequest
from django.utils.deprecation import MiddlewareMixin
from django.utils.translation import ugettext_lazy as _

from readthedocs.core.utils import cname_to_slug
from readthedocs.projects.models import Project, Domain
from readthedocs.projects.models import Domain, Project

log = logging.getLogger(__name__)

Expand All @@ -30,7 +32,7 @@
)


class SubdomainMiddleware(object):
class SubdomainMiddleware(MiddlewareMixin):

"""Middleware to display docs for non-dashboard domains."""

Expand Down Expand Up @@ -138,12 +140,12 @@ def process_response(self, request, response):
return response


class SingleVersionMiddleware(object):
class SingleVersionMiddleware(MiddlewareMixin):

"""
Reset urlconf for requests for 'single_version' docs.

In settings.MIDDLEWARE_CLASSES, SingleVersionMiddleware must follow after
In settings.MIDDLEWARE, SingleVersionMiddleware must follow after
SubdomainMiddleware.
"""

Expand Down Expand Up @@ -192,7 +194,7 @@ def process_request(self, request):


# Forked from old Django
class ProxyMiddleware(object):
class ProxyMiddleware(MiddlewareMixin):

"""
Middleware that sets REMOTE_ADDR based on HTTP_X_FORWARDED_FOR, if the.
Expand Down
34 changes: 34 additions & 0 deletions readthedocs/core/migrations/0005_migrate-old-passwords.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.11.16 on 2018-10-11 17:28
from __future__ import unicode_literals

from django.db import migrations


def forwards_func(apps, schema_editor):
User = apps.get_model('auth', 'User')

old_password_patterns = (
'sha1$',
# RTD's production database doesn't have any of these
# but they are included for completeness
'md5$',
'crypt$',
)
for pattern in old_password_patterns:
users = User.objects.filter(password__startswith=pattern)
for user in users:
user.set_unusable_password()
user.save()


class Migration(migrations.Migration):

dependencies = [
('core', '0004_ad-opt-out'),
('auth', '0008_alter_user_username_max_length'),
]

operations = [
migrations.RunPython(forwards_func),
]
2 changes: 1 addition & 1 deletion readthedocs/projects/fixtures/test_data.json
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@
"last_login": "2010-08-14 01:51:05",
"groups": [],
"user_permissions": [],
"password": "sha1$efaa6$17551368b198ef0dffcbf388908b7a609ec22eb1",
"password": "pbkdf2_sha256$30000$Vs87OlKZEzCb$nUw1o5pGQw7ff/QhnleSpUOupBaT1DogZrVaoZyQRyc=",
"email": "e@etest.co",
"date_joined": "2010-08-14 01:50:58"
}
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,8 @@ def __init__(self, version, attrs=None, check_test=bool):
super(DualCheckboxWidget, self).__init__(attrs, check_test)
self.version = version

def render(self, name, value, attrs=None):
checkbox = super(DualCheckboxWidget, self).render(name, value, attrs)
def render(self, name, value, attrs=None, renderer=None):
checkbox = super(DualCheckboxWidget, self).render(name, value, attrs, renderer)
icon = self.render_icon()
return mark_safe('{}{}'.format(checkbox, icon))

Expand Down
3 changes: 2 additions & 1 deletion readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@ class Meta:

def __init__(self, *args, **kwargs):
self.features = kwargs.pop('features', [])
ad_free = (not kwargs.pop('show_advertising', True))
# These fields only exist on the API return, not on the model, so we'll
# remove them to avoid throwing exceptions due to unexpected fields
for key in ['users', 'resource_uri', 'absolute_url', 'downloads',
Expand All @@ -893,7 +894,7 @@ def __init__(self, *args, **kwargs):
super(APIProject, self).__init__(*args, **kwargs)

# Overwrite the database property with the value from the API
self.ad_free = (not kwargs.pop('show_advertising', True))
self.ad_free = ad_free

def save(self, *args, **kwargs):
return 0
Expand Down
15 changes: 3 additions & 12 deletions readthedocs/rtd_tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,8 @@ def test_get_invalid_raw_log(self):
class APITests(TestCase):
fixtures = ['eric.json', 'test_data.json']

def test_make_project(self):
"""Test that a superuser can use the API."""
def test_cant_make_project(self):
"""Test that a user can't use the API to create projects."""
Copy link
Member

Choose a reason for hiding this comment

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

You inverted the logic of the test here.

Don't we still need the test for testing that it's possible to use the API as super user to create a project?

Copy link
Member

Choose a reason for hiding this comment

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

We decided to remove the creation of a project using the api, it was never documented anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On this change specifically, there's more backstory in #4325.

post_data = {
'name': 'awesome-project',
'repo': 'https://github.com/ericholscher/django-kong.git',
Expand All @@ -430,16 +430,7 @@ def test_make_project(self):
content_type='application/json',
HTTP_AUTHORIZATION='Basic %s' % super_auth,
)
self.assertEqual(resp.status_code, 201)
self.assertEqual(resp['location'], '/api/v1/project/24/')
resp = self.client.get(
'/api/v1/project/24/',
data={'format': 'json'},
HTTP_AUTHORIZATION='Basic %s' % eric_auth,
)
self.assertEqual(resp.status_code, 200)
obj = json.loads(resp.content)
self.assertEqual(obj['slug'], 'awesome-project')
self.assertEqual(resp.status_code, status.HTTP_401_UNAUTHORIZED)

def test_user_doesnt_get_full_api_return(self):
user_normal = get(User, is_staff=False)
Expand Down
2 changes: 1 addition & 1 deletion readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def INSTALLED_APPS(self): # noqa
def USE_PROMOS(self): # noqa
return 'readthedocsext.donate' in self.INSTALLED_APPS

MIDDLEWARE_CLASSES = (
MIDDLEWARE = (
Copy link
Member

Choose a reason for hiding this comment

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

This will require a change in -ops and -corporate-ops.

'readthedocs.core.middleware.ProxyMiddleware',
'readthedocs.core.middleware.FooterNoSessionMiddleware',
'django.middleware.locale.LocaleMiddleware',
Expand Down
6 changes: 3 additions & 3 deletions requirements/pip.txt
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ mkdocs==0.17.3
# Markdown 3.0 breaks with older Django Rest Framework
Markdown<3.0

django==1.9.13
django==1.11.16
six==1.11.0
future==0.16.0

# django-tastypie 0.13.x and 0.14.0 are not compatible with our code
django-tastypie==0.13.0
# django-tastypie 0.14.0 drops support for django 1.9
Copy link
Member

Choose a reason for hiding this comment

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

If we are migrating to django 1.11 we may be possible to use 0.14.x here, I think.

django-tastypie==0.13.3

django-guardian==1.4.9
django-extensions==2.1.2
Expand Down