Django 1.9 compatibility issues #507
Conversation
|
The installation instructions at https://mysociety.github.io/sayit/install/ will also need updating, as the |
dracos
force-pushed
the
1.9-support
branch
2 times, most recently
from
43bc11f to
eef3a3d
Compare
dracos
force-pushed
the
version-1.4
branch
2 times, most recently
from
352b4fd to
28961ca
Compare
dracos
force-pushed
the
1.9-support
branch
2 times, most recently
from
771b9ce to
a434a6b
Compare
dracos
force-pushed
the
1.9-support
branch
3 times, most recently
from
9b0e6a0 to
578954c
Compare
There was a problem hiding this comment.
(n.b. I've just been reviewing this by eye - I haven't tried this latest version myself)
This looks good to me, but there are a few places I've commented on where I think a bit more explanation in the commit message or splitting some commits would be helpful for people looking through the history in the future.
| @@ -176,7 +176,7 @@ def test_add_section_with_heading(self): | |||
| 'heading': 'A test section' | |||
| }) | |||
| new_section = Section.objects.order_by('-id')[0] | |||
| self.assertRedirects(resp, '%s' % new_section.slug) | |||
| self.assertRedirects(resp, '/%s' % new_section.slug) | |||
There was a problem hiding this comment.
Probably a silly question, but why was this working before?
There was a problem hiding this comment.
Before 1.9 it added scheme+host (and presumably therefore the initial /) implicitly. All redirect checks are relative since 1.9.
| @@ -144,7 +144,7 @@ def test_speaker_headshots_in_speeches_section(self): | |||
| def test_create_speaker_with_long_image_url(self): | |||
| sixtyfive = '1234567890' * 6 + '12345' | |||
| ninetynine = '1234567890' * 9 + '123456789' | |||
| long_image_url = 'http://example.com/image%%E2%%97%%8F%s.jpg' % ninetynine | |||
| long_image_url = 'http://example.com/image%%E1%%BD%%A0%s.jpg' % ninetynine | |||
There was a problem hiding this comment.
This confused me a bit - could you possibly explain in the commit message that (presumably) U+25CF BLACK CIRCLE is rejected by get_valid_name() but U+1F60 GREEK SMALL LETTER OMEGA WITH PSILI, while a surprising character to have in a file name, is fine?
| @@ -42,7 +42,7 @@ def read_file(filename): | |||
| 'pytz >= 2013d', | |||
| 'six >= 1.4.1', | |||
| django, | |||
| 'Django-Select2 == 4.3.2', | |||
| 'mysociety-Django-Select2 == 4.3.2.1', | |||
There was a problem hiding this comment.
Maybe this should be in a separate commit with an explanation of why we're using our own fork?
tox.ini
Outdated
|
|
||
| [testenv] | ||
| commands = | ||
| flake8: flake8 | ||
| py: pip install -e .[test] | ||
| py: python manage.py test | ||
| py: python manage.py test {posargs} |
There was a problem hiding this comment.
Why is this needed now, but not before?
dracos
force-pushed
the
1.9-support
branch
3 times, most recently
from
202b6ba to
890326d
Compare
* page_range is now an iterator. * Test redirects need initial slash. (1) * Text fix as uploaded files call get_valid_name(). (2) (1) Django 1.9 started supporting relative redirects, whereas previously it would automatically add scheme and host. Or something like that. (2) This means it strips all non-alphanumeric Unicode characters, and our test was previously using 'black circle' which is now stripped.
Use a fork of Django-Select2 because we are not as yet upgrading to their version 5 rewrite. Switch xip.io to nip.io. Also actually run python 3 tests on python 3!
* Upgrade project to use TEMPLATES setting. * Length function now inbuilt. * current_app on self.request. * Make sure files saved inside MEDIA_ROOT. * All allowed_hosts. * Something tries to access queryset in 1.11. * Fix for Django 1.11 changing behaviour of getlist().
Adds 1.9-1.11 support, makes tests actually run in python 3 (rather than python 2!), Travis container build, selenium switch to geckodriver.