From 3b8f30d223a7f307223bdd1312b0428594c07ef4 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 3 Jun 2015 14:23:58 +0100 Subject: [PATCH 1/6] Moved image field help text/error message generation into __init__ This allows us to override the WAGTAILIMAGES_MAX_UPLOAD_SIZE setting for testing --- wagtail/wagtailimages/fields.py | 86 ++++++++++--------------- wagtail/wagtailimages/views/chooser.py | 3 +- wagtail/wagtailimages/views/images.py | 2 - wagtail/wagtailimages/views/multiple.py | 18 ++---- 4 files changed, 43 insertions(+), 66 deletions(-) diff --git a/wagtail/wagtailimages/fields.py b/wagtail/wagtailimages/fields.py index a674568eb..36351caf7 100644 --- a/wagtail/wagtailimages/fields.py +++ b/wagtail/wagtailimages/fields.py @@ -12,60 +12,44 @@ from django.conf import settings ALLOWED_EXTENSIONS = ['gif', 'jpg', 'jpeg', 'png'] SUPPORTED_FORMATS_TEXT = _("GIF, JPEG, PNG") -INVALID_IMAGE_ERROR = _( - "Not a supported image format. Supported formats: %s." -) % SUPPORTED_FORMATS_TEXT - -INVALID_IMAGE_KNOWN_FORMAT_ERROR = _( - "Not a valid %s image." -) - -MAX_UPLOAD_SIZE = getattr(settings, 'WAGTAILIMAGES_MAX_UPLOAD_SIZE', 10 * 1024 * 1024) - -if MAX_UPLOAD_SIZE is not None: - MAX_UPLOAD_SIZE_TEXT = filesizeformat(MAX_UPLOAD_SIZE) - - FILE_TOO_LARGE_ERROR = _( - "This file is too big. Maximum filesize %(max_upload_size)s." - ) % { - 'max_upload_size': MAX_UPLOAD_SIZE_TEXT, - } - - FILE_TOO_LARGE_KNOWN_SIZE_ERROR = _( - "This file is too big (%%(max_upload_size)s). Maximum filesize %s." - ) % { - 'max_upload_size': MAX_UPLOAD_SIZE_TEXT, - } - - IMAGE_FIELD_HELP_TEXT = _( - "Supported formats: %(supported_formats)s. Maximum filesize: %(max_upload_size)s." - ) % { - 'supported_formats': SUPPORTED_FORMATS_TEXT, - 'max_upload_size': MAX_UPLOAD_SIZE_TEXT, - } -else: - MAX_UPLOAD_SIZE_TEXT = "" - FILE_TOO_LARGE_ERROR = "" - FILE_TOO_LARGE_KNOWN_SIZE_ERROR = "" - - IMAGE_FIELD_HELP_TEXT = _( - "Supported formats: %(supported_formats)s." - ) % { - 'supported_formats': SUPPORTED_FORMATS_TEXT, - } - class WagtailImageField(ImageField): - default_error_messages = { - 'invalid_image': INVALID_IMAGE_ERROR, - 'invalid_image_known_format': INVALID_IMAGE_KNOWN_FORMAT_ERROR, - 'file_too_large': FILE_TOO_LARGE_KNOWN_SIZE_ERROR, - } - def __init__(self, *args, **kwargs): super(WagtailImageField, self).__init__(*args, **kwargs) - self.help_text = IMAGE_FIELD_HELP_TEXT + # Get max upload size from settings + self.max_upload_size = getattr(settings, 'WAGTAILIMAGES_MAX_UPLOAD_SIZE', 10 * 1024 * 1024) + max_upload_size_text = filesizeformat(self.max_upload_size) + + # Help text + if self.max_upload_size is not None: + self.help_text = _( + "Supported formats: %(supported_formats)s. Maximum filesize: %(max_upload_size)s." + ) % { + 'supported_formats': SUPPORTED_FORMATS_TEXT, + 'max_upload_size': max_upload_size_text, + } + else: + self.help_text = _( + "Supported formats: %(supported_formats)s." + ) % { + 'supported_formats': SUPPORTED_FORMATS_TEXT, + } + + # Error messages + self.error_messages['invalid_image'] = _( + "Not a supported image format. Supported formats: %s." + ) % SUPPORTED_FORMATS_TEXT + + self.error_messages['invalid_image_known_format'] = _( + "Not a valid %s image." + ) + + self.error_messages['file_too_large'] = _( + "This file is too big (%%(max_upload_size)s). Maximum filesize %s." + ) % { + 'max_upload_size': max_upload_size_text, + } def check_image_file_format(self, f): # Check file extension @@ -111,11 +95,11 @@ class WagtailImageField(ImageField): def check_image_file_size(self, f): # Upload size checking can be disabled by setting max upload size to None - if MAX_UPLOAD_SIZE is None: + if self.max_upload_size is None: return # Check the filesize - if f.size > MAX_UPLOAD_SIZE: + if f.size > self.max_upload_size: raise ValidationError(self.error_messages['file_too_large'] % ( filesizeformat(f.size), ), code='file_too_large') diff --git a/wagtail/wagtailimages/views/chooser.py b/wagtail/wagtailimages/views/chooser.py index fbfd9bbb2..edc6ac387 100644 --- a/wagtail/wagtailimages/views/chooser.py +++ b/wagtail/wagtailimages/views/chooser.py @@ -12,7 +12,6 @@ from wagtail.wagtailsearch.backends import get_search_backends from wagtail.wagtailimages.models import get_image_model from wagtail.wagtailimages.forms import get_image_form, ImageInsertionForm from wagtail.wagtailimages.formats import get_image_format -from wagtail.wagtailimages.fields import MAX_UPLOAD_SIZE def get_image_json(image): @@ -147,7 +146,7 @@ def chooser_upload(request): return render_modal_workflow( request, 'wagtailimages/chooser/chooser.html', 'wagtailimages/chooser/chooser.js', - {'images': images, 'uploadform': form, 'searchform': searchform, 'max_filesize': MAX_UPLOAD_SIZE} + {'images': images, 'uploadform': form, 'searchform': searchform} ) diff --git a/wagtail/wagtailimages/views/images.py b/wagtail/wagtailimages/views/images.py index 339ee4a8f..eaf00cc96 100644 --- a/wagtail/wagtailimages/views/images.py +++ b/wagtail/wagtailimages/views/images.py @@ -17,7 +17,6 @@ from wagtail.wagtailsearch.backends import get_search_backends from wagtail.wagtailimages.models import get_image_model, Filter from wagtail.wagtailimages.forms import get_image_form, URLGeneratorForm from wagtail.wagtailimages.utils import generate_signature -from wagtail.wagtailimages.fields import MAX_UPLOAD_SIZE from wagtail.wagtailimages.exceptions import InvalidFilterSpecError @@ -252,7 +251,6 @@ def add(request): return render(request, "wagtailimages/images/add.html", { 'form': form, - 'max_filesize': MAX_UPLOAD_SIZE, }) diff --git a/wagtail/wagtailimages/views/multiple.py b/wagtail/wagtailimages/views/multiple.py index 71888a3d2..29eb9ae5d 100644 --- a/wagtail/wagtailimages/views/multiple.py +++ b/wagtail/wagtailimages/views/multiple.py @@ -12,13 +12,7 @@ from wagtail.wagtailsearch.backends import get_search_backends from wagtail.wagtailimages.models import get_image_model from wagtail.wagtailimages.forms import get_image_form -from wagtail.wagtailimages.fields import ( - MAX_UPLOAD_SIZE, - IMAGE_FIELD_HELP_TEXT, - INVALID_IMAGE_ERROR, - ALLOWED_EXTENSIONS, - FILE_TOO_LARGE_ERROR, -) +from wagtail.wagtailimages.fields import ALLOWED_EXTENSIONS from wagtail.utils.compat import render_to_string @@ -87,13 +81,15 @@ def add(request): # https://github.com/django/django/blob/stable/1.6.x/django/forms/util.py#L45 'error_message': '\n'.join(['\n'.join([force_text(i) for i in v]) for k, v in form.errors.items()]), }) + else: + form = ImageForm() return render(request, 'wagtailimages/multiple/add.html', { - 'max_filesize': MAX_UPLOAD_SIZE, - 'help_text': IMAGE_FIELD_HELP_TEXT, + 'max_filesize': form.fields['file'].max_upload_size, + 'help_text': form.fields['file'].help_text, 'allowed_extensions': ALLOWED_EXTENSIONS, - 'error_max_file_size': FILE_TOO_LARGE_ERROR, - 'error_accepted_file_types': INVALID_IMAGE_ERROR, + 'error_max_file_size': form.fields['file'].error_messages['file_too_large'], + 'error_accepted_file_types': form.fields['file'].error_messages['invalid_image'], }) From 2fc58033b7fdbc5ba7b4c4dde4a524d05ed04ef7 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 3 Jun 2015 14:31:40 +0100 Subject: [PATCH 2/6] Failing test for #1334 --- .../wagtailimages/tests/test_admin_views.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index ac0698d21..940d5b544 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -1,6 +1,7 @@ import json +import unittest -from django.test import TestCase +from django.test import TestCase, override_settings from django.utils.http import urlquote from django.core.urlresolvers import reverse from django.contrib.auth.models import Permission @@ -95,6 +96,22 @@ class TestImageAddView(TestCase, WagtailTestUtils): # The form should have an error self.assertFormError(response, 'form', 'file', "This field is required.") + @unittest.expectedFailure + @override_settings(WAGTAILIMAGES_MAX_UPLOAD_SIZE=1) + def test_add_too_large_file(self): + response = self.post({ + 'title': "Test image", + 'file': SimpleUploadedFile('test.png', get_test_image_file().file.getvalue()), + }) + + # Shouldn't redirect anywhere + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailimages/images/add.html') + + # The form should have an error + # Note: \xa0 = non-blocking space + self.assertFormError(response, 'form', 'file', "This file is too big (1.9\xa0KB). Maximum filesize 1\xa0byte.") + class TestImageEditView(TestCase, WagtailTestUtils): def setUp(self): From 8a55446d64a55afed71c2edf1d516e21b773b9af Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 3 Jun 2015 14:52:45 +0100 Subject: [PATCH 3/6] Fixed bad formatted string. Fixes #1334 --- wagtail/wagtailimages/fields.py | 6 ++---- wagtail/wagtailimages/tests/test_admin_views.py | 2 -- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailimages/fields.py b/wagtail/wagtailimages/fields.py index 36351caf7..93c0ac950 100644 --- a/wagtail/wagtailimages/fields.py +++ b/wagtail/wagtailimages/fields.py @@ -46,10 +46,8 @@ class WagtailImageField(ImageField): ) self.error_messages['file_too_large'] = _( - "This file is too big (%%(max_upload_size)s). Maximum filesize %s." - ) % { - 'max_upload_size': max_upload_size_text, - } + "This file is too big (%%s). Maximum filesize %s." + ) % max_upload_size_text def check_image_file_format(self, f): # Check file extension diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index 940d5b544..fa0748d9e 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -1,5 +1,4 @@ import json -import unittest from django.test import TestCase, override_settings from django.utils.http import urlquote @@ -96,7 +95,6 @@ class TestImageAddView(TestCase, WagtailTestUtils): # The form should have an error self.assertFormError(response, 'form', 'file', "This field is required.") - @unittest.expectedFailure @override_settings(WAGTAILIMAGES_MAX_UPLOAD_SIZE=1) def test_add_too_large_file(self): response = self.post({ From e2a9ed39245875c2646bfa91e2bdf42fa8fddf77 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 4 Jun 2015 10:32:42 +0100 Subject: [PATCH 4/6] Fix python 2 --- wagtail/wagtailimages/tests/test_admin_views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index fa0748d9e..10a90941e 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -1,3 +1,5 @@ +from __future__ import unicode_literals + import json from django.test import TestCase, override_settings From 0def3c1768a5d2357f42d04fafe87c08a7cd3aed Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 16 Jun 2015 13:41:33 +0100 Subject: [PATCH 5/6] Fixed max upload size error message in multi image uploader --- wagtail/wagtailimages/fields.py | 4 ++++ wagtail/wagtailimages/tests/test_admin_views.py | 7 +++++++ wagtail/wagtailimages/views/multiple.py | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailimages/fields.py b/wagtail/wagtailimages/fields.py index 93c0ac950..2663d6667 100644 --- a/wagtail/wagtailimages/fields.py +++ b/wagtail/wagtailimages/fields.py @@ -49,6 +49,10 @@ class WagtailImageField(ImageField): "This file is too big (%%s). Maximum filesize %s." ) % max_upload_size_text + self.error_messages['file_too_large_unknown_size'] = _( + "This file is too big. Maximum filesize %s." + ) % max_upload_size_text + def check_image_file_format(self, f): # Check file extension extension = os.path.splitext(f.name)[1].lower()[1:] diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index 10a90941e..7c7809ebe 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -302,6 +302,13 @@ class TestMultipleImageUploader(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailimages/multiple/add.html') + @override_settings(WAGTAILIMAGES_MAX_UPLOAD_SIZE=1000) + def test_add_max_file_size_context_variables(self): + response = self.client.get(reverse('wagtailimages_add_multiple')) + + self.assertEqual(response.context['max_filesize'], 1000) + self.assertEqual(response.context['error_max_file_size'], "This file is too big. Maximum filesize 1000\xa0bytes.") + def test_add_post(self): """ This tests that a POST request to the add view saves the image and returns an edit form diff --git a/wagtail/wagtailimages/views/multiple.py b/wagtail/wagtailimages/views/multiple.py index 29eb9ae5d..13142aa44 100644 --- a/wagtail/wagtailimages/views/multiple.py +++ b/wagtail/wagtailimages/views/multiple.py @@ -88,7 +88,7 @@ def add(request): 'max_filesize': form.fields['file'].max_upload_size, 'help_text': form.fields['file'].help_text, 'allowed_extensions': ALLOWED_EXTENSIONS, - 'error_max_file_size': form.fields['file'].error_messages['file_too_large'], + 'error_max_file_size': form.fields['file'].error_messages['file_too_large_unknown_size'], 'error_accepted_file_types': form.fields['file'].error_messages['invalid_image'], }) From 0d54e5b2e7b2ae81c3c96d466a03c2381c7d241f Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 16 Jun 2015 14:54:45 +0100 Subject: [PATCH 6/6] Don't hard code image file size in test --- wagtail/wagtailimages/tests/test_admin_views.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailimages/tests/test_admin_views.py b/wagtail/wagtailimages/tests/test_admin_views.py index 7c7809ebe..8a4b954bb 100644 --- a/wagtail/wagtailimages/tests/test_admin_views.py +++ b/wagtail/wagtailimages/tests/test_admin_views.py @@ -7,6 +7,7 @@ from django.utils.http import urlquote from django.core.urlresolvers import reverse from django.contrib.auth.models import Permission from django.core.files.uploadedfile import SimpleUploadedFile +from django.template.defaultfilters import filesizeformat # Get the chars that Django considers safe to leave unescaped in a URL # This list changed in Django 1.8: https://github.com/django/django/commit/e167e96cfea670422ca75d0b35fe7c4195f25b63 @@ -99,9 +100,11 @@ class TestImageAddView(TestCase, WagtailTestUtils): @override_settings(WAGTAILIMAGES_MAX_UPLOAD_SIZE=1) def test_add_too_large_file(self): + file_content = get_test_image_file().file.getvalue() + response = self.post({ 'title': "Test image", - 'file': SimpleUploadedFile('test.png', get_test_image_file().file.getvalue()), + 'file': SimpleUploadedFile('test.png', file_content), }) # Shouldn't redirect anywhere @@ -109,8 +112,10 @@ class TestImageAddView(TestCase, WagtailTestUtils): self.assertTemplateUsed(response, 'wagtailimages/images/add.html') # The form should have an error - # Note: \xa0 = non-blocking space - self.assertFormError(response, 'form', 'file', "This file is too big (1.9\xa0KB). Maximum filesize 1\xa0byte.") + self.assertFormError(response, 'form', 'file', "This file is too big ({file_size}). Maximum filesize {max_file_size}.".format( + file_size=filesizeformat(len(file_content)), + max_file_size=filesizeformat(1), + )) class TestImageEditView(TestCase, WagtailTestUtils):