From 2f029fc336510ab40fc9cb29c3eaf74e3f252134 Mon Sep 17 00:00:00 2001 From: Ben Margolis Date: Wed, 4 Jun 2014 11:28:14 -0700 Subject: [PATCH 01/48] adding custom img attrs to image_tags, but no custom TestCasecreated for it --- wagtail/wagtailimages/models.py | 5 ++- .../templates/wagtailimages/imagetag.html | 1 + .../wagtailimages/templatetags/image_tags.py | 41 +++++++++++++------ 3 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 wagtail/wagtailimages/templates/wagtailimages/imagetag.html diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 24fd1f0bf..bd2fc8751 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -12,6 +12,7 @@ from django.utils.safestring import mark_safe from django.utils.html import escape from django.conf import settings from django.utils.translation import ugettext_lazy as _ +from django.template.loader import render_to_string from unidecode import unidecode @@ -225,9 +226,9 @@ class AbstractRendition(models.Model): def url(self): return self.file.url - def img_tag(self): + def img_tag(self, attrs={}): return mark_safe( - '%s' % (escape(self.url), self.width, self.height, escape(self.image.title)) + render_to_string('wagtailimages/imagetag.html',{'self' : self, 'attrs' : attrs, }) ) class Meta: diff --git a/wagtail/wagtailimages/templates/wagtailimages/imagetag.html b/wagtail/wagtailimages/templates/wagtailimages/imagetag.html new file mode 100644 index 000000000..d10496739 --- /dev/null +++ b/wagtail/wagtailimages/templates/wagtailimages/imagetag.html @@ -0,0 +1 @@ +{{ self.image.title }} \ No newline at end of file diff --git a/wagtail/wagtailimages/templatetags/image_tags.py b/wagtail/wagtailimages/templatetags/image_tags.py index e59d9cd14..48800d920 100644 --- a/wagtail/wagtailimages/templatetags/image_tags.py +++ b/wagtail/wagtailimages/templatetags/image_tags.py @@ -7,32 +7,46 @@ register = template.Library() # Local cache of filters, avoid hitting the DB filters = {} + @register.tag(name="image") def image(parser, token): - args = token.split_contents() + bits = token.split_contents()[1:] + image_var = bits[0] + filter_spec = bits[1] + bits = bits[2:] - if len(args) == 3: + if len(bits) == 0: # token is of the form {% image self.photo max-320x200 %} - tag_name, image_var, filter_spec = args return ImageNode(image_var, filter_spec) - elif len(args) == 5: + elif len(bits) == 2: # token is of the form {% image self.photo max-320x200 as img %} - tag_name, image_var, filter_spec, as_token, out_var = args - if as_token != 'as': - raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 %%} or {%% image self.photo max-320x200 as img %%}") + if bits[0] == 'as': + return ImageNode(image_var, filter_spec, output_var_name=bits[1]) - return ImageNode(image_var, filter_spec, out_var) + if len(bits) > 0: + # customized attrs + attrs = {} + for bit in bits: + try: + name,value = bit.split('=') + except: + raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 [ custom-attr=\"value\" [ ... ] ] %%} or {%% image self.photo max-320x200 as img %%}") + if name + attrs[name] = parser.compile_filter(value) # setup to resolve context variables as value - else: - raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 %%} or {%% image self.photo max-320x200 as img %%}") + return ImageNode(image_var, filter_spec, attrs=attrs) + + # something is wrong if we made it this far + raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 [ custom-attr=\"value\" [ ... ] ] %%} or {%% image self.photo max-320x200 as img %%}") class ImageNode(template.Node): - def __init__(self, image_var_name, filter_spec, output_var_name=None): + def __init__(self, image_var_name, filter_spec, output_var_name=None, attrs={}): self.image_var = template.Variable(image_var_name) self.output_var_name = output_var_name + self.attrs = attrs if filter_spec not in filters: filters[filter_spec], _ = Filter.objects.get_or_create(spec=filter_spec) @@ -66,4 +80,7 @@ class ImageNode(template.Node): return '' else: # render the rendition's image tag now - return rendition.img_tag() + resolved_attrs = {} + for key in self.attrs: + resolved_attrs[key] = self.attrs[key].resolve(context) + return rendition.img_tag(resolved_attrs) From f70768cc4e65dd07bf51629a07387f9dd6c959e0 Mon Sep 17 00:00:00 2001 From: Ben Margolis Date: Wed, 4 Jun 2014 12:52:17 -0700 Subject: [PATCH 02/48] typo --- wagtail/wagtailimages/templatetags/image_tags.py | 1 - 1 file changed, 1 deletion(-) diff --git a/wagtail/wagtailimages/templatetags/image_tags.py b/wagtail/wagtailimages/templatetags/image_tags.py index 48800d920..e8a4c2191 100644 --- a/wagtail/wagtailimages/templatetags/image_tags.py +++ b/wagtail/wagtailimages/templatetags/image_tags.py @@ -33,7 +33,6 @@ def image(parser, token): name,value = bit.split('=') except: raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 [ custom-attr=\"value\" [ ... ] ] %%} or {%% image self.photo max-320x200 as img %%}") - if name attrs[name] = parser.compile_filter(value) # setup to resolve context variables as value return ImageNode(image_var, filter_spec, attrs=attrs) From cf3d6a4e66b646bf666b73fa0b16f9f2f5418c5b Mon Sep 17 00:00:00 2001 From: Jeffrey Hearn Date: Fri, 6 Jun 2014 10:58:14 -0400 Subject: [PATCH 03/48] Addition of attrs property to image template tag --- wagtail/wagtailimages/models.py | 6 ++++++ wagtail/wagtailimages/tests.py | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 24fd1f0bf..921f65921 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -225,6 +225,12 @@ class AbstractRendition(models.Model): def url(self): return self.file.url + @property + def attrs(self): + return mark_safe( + 'src="%s" width="%d" height="%d" alt="%s"' % (escape(self.url), self.width, self.height, escape(self.image.title)) + ) + def img_tag(self): return mark_safe( '%s' % (escape(self.url), self.width, self.height, escape(self.image.title)) diff --git a/wagtail/wagtailimages/tests.py b/wagtail/wagtailimages/tests.py index 00d1245e6..f36751650 100644 --- a/wagtail/wagtailimages/tests.py +++ b/wagtail/wagtailimages/tests.py @@ -183,6 +183,18 @@ class TestImageTag(TestCase): self.assertTrue('height="300"' in result) self.assertTrue('alt="Test image"' in result) + def render_image_tag_as(self, image, filter_spec): + temp = template.Template('{% load image_tags %}{% image image_obj ' + filter_spec + ' as test_img %}') + context = template.Context({'image_obj': image}) + return temp.render(context) + + def test_image_tag_attrs(self): + result = self.render_image_tag_as(self.image, 'width-400') + + # Check that all the required HTML attributes are set + self.assertTrue('width="400"' in result) + self.assertTrue('height="300"' in result) + self.assertTrue('alt="Test image"' in result) ## ===== ADMIN VIEWS ===== From b480c9e7eb9a88d99402ced8b5535b51d2f78f17 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 17 Jun 2014 17:25:49 +0100 Subject: [PATCH 04/48] Fixes to wagtailadmin page index view --- wagtail/wagtailadmin/views/pages.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 69e6b7f4f..7efe553ce 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -25,16 +25,14 @@ def index(request, parent_page_id=None): pages = parent_page.get_children().prefetch_related('content_type') # Get page ordering - if 'ordering' in request.GET: - ordering = request.GET['ordering'] - - if ordering in ['title', '-title', 'content_type', '-content_type', 'live', '-live']: - pages = pages.order_by(ordering) - else: + ordering = request.GET.get('ordering', 'title') + if ordering not in ['title', '-title', 'content_type', '-content_type', 'live', '-live', 'ord']: ordering = 'title' # Pagination if ordering != 'ord': + pages = pages.order_by(ordering) + p = request.GET.get('p', 1) paginator = Paginator(pages, 50) try: From 13c341a4d4f243acc52023fbcff316d7b607ae8c Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 17 Jun 2014 17:26:31 +0100 Subject: [PATCH 05/48] Improvements to wagtailadmin explorer tests --- .../wagtailadmin/tests/test_pages_views.py | 80 ++++++++++++++++++- 1 file changed, 77 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 12c42f3e4..878441f88 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -5,6 +5,7 @@ from wagtail.wagtailcore.models import Page, PageRevision from django.core.urlresolvers import reverse from django.contrib.auth.models import User, Permission from django.core import mail +from django.core.paginator import Paginator class TestPageExplorer(TestCase, WagtailTestUtils): @@ -13,9 +14,10 @@ class TestPageExplorer(TestCase, WagtailTestUtils): self.root_page = Page.objects.get(id=2) # Add child page - self.child_page = SimplePage() - self.child_page.title = "Hello world!" - self.child_page.slug = "hello-world" + self.child_page = SimplePage( + title="Hello world!", + slug="hello-world", + ) self.root_page.add_child(instance=self.child_page) # Login @@ -24,9 +26,81 @@ class TestPageExplorer(TestCase, WagtailTestUtils): def test_explore(self): response = self.client.get(reverse('wagtailadmin_explore', args=(self.root_page.id, ))) self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') self.assertEqual(self.root_page, response.context['parent_page']) self.assertTrue(response.context['pages'].paginator.object_list.filter(id=self.child_page.id).exists()) + def test_explore_root(self): + response = self.client.get(reverse('wagtailadmin_explore_root')) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + self.assertEqual(Page.objects.get(id=1), response.context['parent_page']) + self.assertTrue(response.context['pages'].paginator.object_list.filter(id=self.root_page.id).exists()) + + def test_ordering(self): + response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'content_type'}) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + self.assertEqual(response.context['ordering'], 'content_type') + + def test_invalid_ordering(self): + response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'invalid_order'}) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + self.assertEqual(response.context['ordering'], 'title') + + def test_reordering(self): + response = self.client.get(reverse('wagtailadmin_explore_root'), {'ordering': 'ord'}) + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + self.assertEqual(response.context['ordering'], 'ord') + + # Pages must not be paginated + self.assertNotIsInstance(response.context['pages'], Paginator) + + def make_pages(self): + for i in range(150): + self.root_page.add_child(instance=SimplePage( + title="Page " + str(i), + slug="page-" + str(i), + )) + + def test_pagination(self): + self.make_pages() + + response = self.client.get(reverse('wagtailadmin_explore', args=(self.root_page.id, )), {'p': 2}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + + # Check that we got the correct page + self.assertEqual(response.context['pages'].number, 2) + + def test_pagination_invalid(self): + self.make_pages() + + response = self.client.get(reverse('wagtailadmin_explore', args=(self.root_page.id, )), {'p': 'Hello World!'}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + + # Check that we got page one + self.assertEqual(response.context['pages'].number, 1) + + def test_pagination_out_of_range(self): + self.make_pages() + + response = self.client.get(reverse('wagtailadmin_explore', args=(self.root_page.id, )), {'p': 99999}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/pages/index.html') + + # Check that we got the last page + self.assertEqual(response.context['pages'].number, response.context['pages'].paginator.num_pages) + class TestPageCreation(TestCase, WagtailTestUtils): def setUp(self): From 9917d777c558ef18bc27f25131faa617f84912a6 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 17 Jun 2014 17:38:00 +0100 Subject: [PATCH 06/48] Improvements to existing slug unit tests for create and edit views --- .../wagtailadmin/tests/test_pages_views.py | 28 ++++++++++++++++++- wagtail/wagtailadmin/views/pages.py | 2 ++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 878441f88..de5415d73 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -218,7 +218,7 @@ class TestPageCreation(TestCase, WagtailTestUtils): self.assertEqual(mail.outbox[0].to, ['moderator@email.com']) self.assertEqual(mail.outbox[0].subject, 'The page "New page!" has been submitted for moderation') - def test_create_simplepage_post_existingslug(self): + def test_create_simplepage_post_existing_slug(self): # This tests the existing slug checking on page save # Create a page @@ -239,6 +239,9 @@ class TestPageCreation(TestCase, WagtailTestUtils): # Should not be redirected (as the save should fail) self.assertEqual(response.status_code, 200) + # Check that a form error was raised + self.assertFormError(response, 'form', 'slug', "This slug is already in use") + def test_create_nonexistantparent(self): response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', 100000))) self.assertEqual(response.status_code, 404) @@ -371,6 +374,29 @@ class TestPageEdit(TestCase, WagtailTestUtils): self.assertEqual(mail.outbox[0].to, ['moderator@email.com']) self.assertEqual(mail.outbox[0].subject, 'The page "Hello world!" has been submitted for moderation') # Note: should this be "I've been edited!"? + def test_page_edit_post_existing_slug(self): + # This tests the existing slug checking on page edit + + # Create a page + self.child_page = SimplePage() + self.child_page.title = "Hello world 2" + self.child_page.slug = "hello-world2" + self.root_page.add_child(instance=self.child_page) + + # Attempt to change the slug to one thats already in use + post_data = { + 'title': "Hello world 2", + 'slug': 'hello-world', + 'action-submit': "Submit", + } + response = self.client.post(reverse('wagtailadmin_pages_edit', args=(self.child_page.id, )), post_data) + + # Should not be redirected (as the save should fail) + self.assertEqual(response.status_code, 200) + + # Check that a form error was raised + self.assertFormError(response, 'form', 'slug', "This slug is already in use") + def test_preview_on_edit(self): post_data = { 'title': "I've been edited!", diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 7efe553ce..a611a0647 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -179,6 +179,7 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_ 'parent_page': parent_page, 'edit_handler': edit_handler, 'display_modes': page.get_page_modes(), + 'form': form, # Used in unit tests }) @@ -264,6 +265,7 @@ def edit(request, page_id): 'edit_handler': edit_handler, 'errors_debug': errors_debug, 'display_modes': page.get_page_modes(), + 'form': form, # Used in unit tests }) From 015f7c126d973175edf6e6d18bd277e152be01ac Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 17 Jun 2014 17:49:52 +0100 Subject: [PATCH 07/48] Removed assertURLEqual, replaced with Djangos built in assertRedirects --- wagtail/tests/utils.py | 18 ----------- .../tests/test_account_management.py | 23 +++++-------- .../wagtailadmin/tests/test_pages_views.py | 32 +++++++------------ wagtail/wagtailimages/tests.py | 9 ++---- wagtail/wagtailredirects/tests.py | 9 ++---- wagtail/wagtailsnippets/tests.py | 9 ++---- wagtail/wagtailusers/tests.py | 6 ++-- 7 files changed, 30 insertions(+), 76 deletions(-) diff --git a/wagtail/tests/utils.py b/wagtail/tests/utils.py index 0b13e5932..02327799b 100644 --- a/wagtail/tests/utils.py +++ b/wagtail/tests/utils.py @@ -24,21 +24,3 @@ class WagtailTestUtils(object): self.client.login(username='test', password='password') return user - - # From: https://github.com/django/django/blob/255449c1ee61c14778658caae8c430fa4d76afd6/django/contrib/auth/tests/test_views.py#L70-L85 - def assertURLEqual(self, url, expected, parse_qs=False): - """ - Given two URLs, make sure all their components (the ones given by - urlparse) are equal, only comparing components that are present in both - URLs. - If `parse_qs` is True, then the querystrings are parsed with QueryDict. - This is useful if you don't want the order of parameters to matter. - Otherwise, the query strings are compared as-is. - """ - fields = ParseResult._fields - - for attr, x, y in zip(fields, urlparse(url), urlparse(expected)): - if parse_qs and attr == 'query': - x, y = QueryDict(x), QueryDict(y) - if x and y and x != y: - self.fail("%r != %r (%s doesn't match)" % (url, expected, attr)) diff --git a/wagtail/wagtailadmin/tests/test_account_management.py b/wagtail/wagtailadmin/tests/test_account_management.py index 3553b8fba..566d68723 100644 --- a/wagtail/wagtailadmin/tests/test_account_management.py +++ b/wagtail/wagtailadmin/tests/test_account_management.py @@ -43,8 +43,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_login'), post_data) # Check that the user was redirected to the dashboard - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_home')) # Check that the user was logged in self.assertTrue('_auth_user_id' in self.client.session) @@ -60,8 +59,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): response = self.client.get(reverse('wagtailadmin_login')) # Check that the user was redirected to the dashboard - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_home')) def test_logout(self): """ @@ -71,8 +69,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): response = self.client.get(reverse('wagtailadmin_logout')) # Check that the user was redirected to the login page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_login')) + self.assertRedirects(response, reverse('wagtailadmin_login')) # Check that the user was logged out self.assertFalse('_auth_user_id' in self.client.session) @@ -89,8 +86,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): response = self.client.get(reverse('wagtailadmin_home')) # Check that the user was redirected to the login page and that next was set correctly - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) def test_not_logged_in_redirect_default_settings(self): """ @@ -109,7 +105,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): # Note: The user will be redirected to 'django.contrib.auth.views.login' but # this must be the same URL as 'wagtailadmin_login' self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) class TestAccountSection(TestCase, WagtailTestUtils): @@ -154,8 +150,7 @@ class TestAccountSection(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_account_change_password'), post_data) # Check that the user was redirected to the account page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_account')) + self.assertRedirects(response, reverse('wagtailadmin_account')) # Check that the password was changed self.assertTrue(User.objects.get(username='test').check_password('newpassword')) @@ -214,8 +209,7 @@ class TestPasswordReset(TestCase, WagtailTestUtils): response = self.client.post(reverse('password_reset'), post_data) # Check that the user was redirected to the done page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('password_reset_done')) + self.assertRedirects(response, reverse('password_reset_done')) # Check that a password reset email was sent to the user self.assertEqual(len(mail.outbox), 1) @@ -306,8 +300,7 @@ class TestPasswordReset(TestCase, WagtailTestUtils): response = self.client.post(reverse('password_reset_confirm', kwargs=self.url_kwargs), post_data) # Check that the user was redirected to the complete page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('password_reset_complete')) + self.assertRedirects(response, reverse('password_reset_complete')) # Check that the password was changed self.assertTrue(User.objects.get(username='test').check_password('newpassword')) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index de5415d73..3da08de13 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -159,8 +159,7 @@ class TestPageCreation(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.root_page.id)), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Find the page and check it page = Page.objects.get(path__startswith=self.root_page.path, slug='hello-world').specific @@ -178,8 +177,7 @@ class TestPageCreation(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.root_page.id)), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Find the page and check it page = Page.objects.get(path__startswith=self.root_page.path, slug='hello-world').specific @@ -201,8 +199,7 @@ class TestPageCreation(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.root_page.id)), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Find the page and check it page = Page.objects.get(path__startswith=self.root_page.path, slug='hello-world').specific @@ -317,8 +314,7 @@ class TestPageEdit(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_edit', args=(self.child_page.id, )), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # The page should have "has_unpublished_changes" flag set child_page_new = SimplePage.objects.get(id=self.child_page.id) @@ -335,8 +331,7 @@ class TestPageEdit(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_edit', args=(self.child_page.id, )), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Check that the page was edited child_page_new = SimplePage.objects.get(id=self.child_page.id) @@ -359,8 +354,7 @@ class TestPageEdit(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_edit', args=(self.child_page.id, )), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # The page should have "has_unpublished_changes" flag set child_page_new = SimplePage.objects.get(id=self.child_page.id) @@ -449,8 +443,7 @@ class TestPageDelete(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailadmin_pages_delete', args=(self.child_page.id, )), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Check that the page is gone self.assertEqual(Page.objects.filter(path__startswith=self.root_page.path, slug='hello-world').count(), 0) @@ -609,9 +602,8 @@ class TestPageUnpublish(TestCase, WagtailTestUtils): 'foo': "Must post something or the view won't see this as a POST request", }) - # Check that the user was redirected to the explore page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) + # Should be redirected to explorer page + self.assertRedirects(response, reverse('wagtailadmin_explore', args=(self.root_page.id, ))) # Check that the page was unpublished self.assertFalse(SimplePage.objects.get(id=self.page.id).live) @@ -649,8 +641,7 @@ class TestApproveRejectModeration(TestCase, WagtailTestUtils): }) # Check that the user was redirected to the dashboard - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_home')) # Page must be live self.assertTrue(Page.objects.get(id=self.page.id).live) @@ -701,8 +692,7 @@ class TestApproveRejectModeration(TestCase, WagtailTestUtils): }) # Check that the user was redirected to the dashboard - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailadmin_home')) + self.assertRedirects(response, reverse('wagtailadmin_home')) # Page must not be live self.assertFalse(Page.objects.get(id=self.page.id).live) diff --git a/wagtail/wagtailimages/tests.py b/wagtail/wagtailimages/tests.py index b31e33745..4604a94ef 100644 --- a/wagtail/wagtailimages/tests.py +++ b/wagtail/wagtailimages/tests.py @@ -253,8 +253,7 @@ class TestImageAddView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailimages_index')) + self.assertRedirects(response, reverse('wagtailimages_index')) # Check that the image was created images = Image.objects.filter(title="Test image") @@ -293,8 +292,7 @@ class TestImageEditView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailimages_index')) + self.assertRedirects(response, reverse('wagtailimages_index')) # Check that the image was edited image = Image.objects.get(id=self.image.id) @@ -328,8 +326,7 @@ class TestImageDeleteView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailimages_index')) + self.assertRedirects(response, reverse('wagtailimages_index')) # Check that the image was deleted images = Image.objects.filter(title="Test image") diff --git a/wagtail/wagtailredirects/tests.py b/wagtail/wagtailredirects/tests.py index 29ad7ec0b..9aab1e98a 100644 --- a/wagtail/wagtailredirects/tests.py +++ b/wagtail/wagtailredirects/tests.py @@ -113,8 +113,7 @@ class TestRedirectsAddView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailredirects_index')) + self.assertRedirects(response, reverse('wagtailredirects_index')) # Check that the redirect was created redirects = models.Redirect.objects.filter(old_path='/test') @@ -163,8 +162,7 @@ class TestRedirectsEditView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailredirects_index')) + self.assertRedirects(response, reverse('wagtailredirects_index')) # Check that the redirect was edited redirects = models.Redirect.objects.filter(old_path='/test') @@ -210,8 +208,7 @@ class TestRedirectsDeleteView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailredirects_index')) + self.assertRedirects(response, reverse('wagtailredirects_index')) # Check that the redirect was deleted redirects = models.Redirect.objects.filter(old_path='/test') diff --git a/wagtail/wagtailsnippets/tests.py b/wagtail/wagtailsnippets/tests.py index 9e2587794..1b1e012de 100644 --- a/wagtail/wagtailsnippets/tests.py +++ b/wagtail/wagtailsnippets/tests.py @@ -70,8 +70,7 @@ class TestSnippetCreateView(TestCase, WagtailTestUtils): def test_create(self): response = self.post(post_data={'text': 'test_advert', 'url': 'http://www.example.com/'}) - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailsnippets_list', args=('tests', 'advert'))) + self.assertRedirects(response, reverse('wagtailsnippets_list', args=('tests', 'advert'))) snippets = Advert.objects.filter(text='test_advert') self.assertEqual(snippets.count(), 1) @@ -120,8 +119,7 @@ class TestSnippetEditView(TestCase, WagtailTestUtils): def test_edit(self): response = self.post(post_data={'text': 'edited_test_advert', 'url': 'http://www.example.com/edited'}) - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailsnippets_list', args=('tests', 'advert'))) + self.assertRedirects(response, reverse('wagtailsnippets_list', args=('tests', 'advert'))) snippets = Advert.objects.filter(text='edited_test_advert') self.assertEqual(snippets.count(), 1) @@ -146,8 +144,7 @@ class TestSnippetDelete(TestCase, WagtailTestUtils): response = self.client.post(reverse('wagtailsnippets_delete', args=('tests', 'advert', self.test_snippet.id, )), post_data) # Should be redirected to explorer page - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailsnippets_list', args=('tests', 'advert'))) + self.assertRedirects(response, reverse('wagtailsnippets_list', args=('tests', 'advert'))) # Check that the page is gone self.assertEqual(Advert.objects.filter(text='test_advert').count(), 0) diff --git a/wagtail/wagtailusers/tests.py b/wagtail/wagtailusers/tests.py index 6de97f202..a360fe468 100644 --- a/wagtail/wagtailusers/tests.py +++ b/wagtail/wagtailusers/tests.py @@ -54,8 +54,7 @@ class TestUserCreateView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailusers_index')) + self.assertRedirects(response, reverse('wagtailusers_index')) # Check that the user was created users = User.objects.filter(username='testuser') @@ -96,8 +95,7 @@ class TestUserEditView(TestCase, WagtailTestUtils): }) # Should redirect back to index - self.assertEqual(response.status_code, 302) - self.assertURLEqual(response.url, reverse('wagtailusers_index')) + self.assertRedirects(response, reverse('wagtailusers_index')) # Check that the user was edited user = User.objects.get(id=self.test_user.id) From a713895440657907fd3e39cfafda3a7ad8daf4df Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:14:23 +0100 Subject: [PATCH 08/48] get_page_types no longer required in wagtailadmin.views.pages --- wagtail/wagtailadmin/views/pages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 916c373eb..c60a1800e 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -12,7 +12,7 @@ from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList from wagtail.wagtailadmin.forms import SearchForm from wagtail.wagtailadmin import tasks, hooks -from wagtail.wagtailcore.models import Page, PageRevision, get_page_types +from wagtail.wagtailcore.models import Page, PageRevision @permission_required('wagtailadmin.access_admin') From a4b60715b99135ffe7bc452d312f043125a1d685 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:27:11 +0100 Subject: [PATCH 09/48] Check that the content type passed to wagtailadmin.pages.create is valid according to subpage_types --- wagtail/wagtailadmin/tests/test_pages_views.py | 13 +++++++++++++ wagtail/wagtailadmin/views/pages.py | 12 ++++-------- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 09e6b6f5c..986bb260c 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -719,3 +719,16 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertNotContains(response, 'Standard Child') self.assertEqual(0, len(response.context['page_types'])) + + def test_cannot_add_invalid_subpage_type(self): + # cannot add SimplePage as a child of BusinessIndex, as SimplePage is not present in subpage_types + response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_index.id))) + self.assertEqual(response.status_code, 403) + + # likewise for BusinessChild which has an empty subpage_types list + response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.business_child.id))) + self.assertEqual(response.status_code, 403) + + # but we can add a BusinessChild to BusinessIndex + response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.business_index.id))) + self.assertEqual(response.status_code, 200) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index c60a1800e..559adb922 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -111,15 +111,11 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_ except ContentType.DoesNotExist: raise Http404 - page_class = content_type.model_class() - # page must be in the list of allowed subpage types for this parent ID - # == Restriction temporarily relaxed so that as superusers we can add index pages and things - - # == TODO: reinstate this for regular editors when we have distinct user types - # - # if page_class not in parent_page.clean_subpage_types(): - # messages.error(request, "Sorry, you do not have access to create a page of type '%s' here." % content_type.name) - # return redirect('wagtailadmin_pages_select_type') + if content_type not in parent_page.clean_subpage_types(): + raise PermissionDenied + + page_class = content_type.model_class() page = page_class(owner=request.user) edit_handler_class = get_page_edit_handler(page_class) From 6bfe82f5e59ca375725b2769dc5b2564d3d19e2c Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 22:53:54 +0100 Subject: [PATCH 10/48] Make can_add_subpage and can_publish_subpage permission checks return False for page models that disallow subpages; this ensures that 'add child page' links are not shown --- .../wagtailadmin/tests/test_pages_views.py | 32 ++++++++++++++++--- wagtail/wagtailcore/models.py | 7 +++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 986bb260c..5a2958fb5 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -703,22 +703,44 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): self.login() def test_standard_subpage(self): - response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, ))) + add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, )) + + # explorer should contain a link to 'add child page' + response = self.client.get(reverse('wagtailadmin_explore', args=(self.standard_index.id, ))) + self.assertEqual(response.status_code, 200) + self.assertContains(response, add_subpage_url) + + # add_subpage should give us the full set of page types to choose + response = self.client.get(add_subpage_url) self.assertEqual(response.status_code, 200) self.assertContains(response, 'Standard Child') self.assertContains(response, 'Business Child') def test_business_subpage(self): - response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_index.id, ))) + add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_index.id, )) + + # explorer should contain a link to 'add child page' + response = self.client.get(reverse('wagtailadmin_explore', args=(self.business_index.id, ))) + self.assertEqual(response.status_code, 200) + self.assertContains(response, add_subpage_url) + + # add_subpage should give us a cut-down set of page types to choose + response = self.client.get(add_subpage_url) self.assertEqual(response.status_code, 200) self.assertNotContains(response, 'Standard Child') self.assertContains(response, 'Business Child') def test_business_child_subpage(self): - response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, ))) + add_subpage_url = reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, )) + + # explorer should not contain a link to 'add child page', as this page doesn't accept subpages + response = self.client.get(reverse('wagtailadmin_explore', args=(self.business_child.id, ))) self.assertEqual(response.status_code, 200) - self.assertNotContains(response, 'Standard Child') - self.assertEqual(0, len(response.context['page_types'])) + self.assertNotContains(response, add_subpage_url) + + # this also means that fetching add_subpage is blocked at the permission-check level + response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_child.id, ))) + self.assertEqual(response.status_code, 403) def test_cannot_add_invalid_subpage_type(self): # cannot add SimplePage as a child of BusinessIndex, as SimplePage is not present in subpage_types diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 5c3f9d1c5..d7d23c988 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -843,6 +843,8 @@ class PagePermissionTester(object): def can_add_subpage(self): if not self.user.is_active: return False + if not self.page.specific_class.clean_subpage_types(): # this page model has an empty subpage_types list, so no subpages are allowed + return False return self.user.is_superuser or ('add' in self.permissions) def can_edit(self): @@ -897,10 +899,13 @@ class PagePermissionTester(object): """ Niggly special case for creating and publishing a page in one go. Differs from can_publish in that we want to be able to publish subpages of root, but not - to be able to publish root itself + to be able to publish root itself. (Also, can_publish_subpage returns false if the page + does not allow subpages at all.) """ if not self.user.is_active: return False + if not self.page.specific_class.clean_subpage_types(): # this page model has an empty subpage_types list, so no subpages are allowed + return False return self.user.is_superuser or ('publish' in self.permissions) From eddb060c8d3ea0701fc7ff651127a0f6611eb295 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 17 Jun 2014 23:08:30 +0100 Subject: [PATCH 11/48] Bypass 'choose a page type' screen when there is only one available choice in subpage_types --- wagtail/tests/models.py | 3 +++ .../wagtailadmin/tests/test_pages_views.py | 19 +++++++++++++++---- wagtail/wagtailadmin/views/pages.py | 6 ++++++ 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/wagtail/tests/models.py b/wagtail/tests/models.py index f798129dc..ca6d411da 100644 --- a/wagtail/tests/models.py +++ b/wagtail/tests/models.py @@ -303,6 +303,9 @@ class StandardChild(Page): pass class BusinessIndex(Page): + subpage_types = ['tests.BusinessChild', 'tests.BusinessSubIndex'] + +class BusinessSubIndex(Page): subpage_types = ['tests.BusinessChild'] class BusinessChild(Page): diff --git a/wagtail/wagtailadmin/tests/test_pages_views.py b/wagtail/wagtailadmin/tests/test_pages_views.py index 5a2958fb5..5f4b0021c 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -1,5 +1,5 @@ from django.test import TestCase -from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild +from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild, BusinessSubIndex from wagtail.tests.utils import unittest, WagtailTestUtils from wagtail.wagtailcore.models import Page, PageRevision from django.core.urlresolvers import reverse @@ -681,24 +681,30 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): # Find root page self.root_page = Page.objects.get(id=2) - # Add standard page + # Add standard page (allows subpages of any type) self.standard_index = StandardIndex() self.standard_index.title = "Standard Index" self.standard_index.slug = "standard-index" self.root_page.add_child(instance=self.standard_index) - # Add business page + # Add business page (allows BusinessChild and BusinessSubIndex as subpages) self.business_index = BusinessIndex() self.business_index.title = "Business Index" self.business_index.slug = "business-index" self.root_page.add_child(instance=self.business_index) - # Add business child + # Add business child (allows no subpages) self.business_child = BusinessChild() self.business_child.title = "Business Child" self.business_child.slug = "business-child" self.business_index.add_child(instance=self.business_child) + # Add business subindex (allows only BusinessChild as subpages) + self.business_subindex = BusinessSubIndex() + self.business_subindex.title = "Business Subindex" + self.business_subindex.slug = "business-subindex" + self.business_index.add_child(instance=self.business_subindex) + # Login self.login() @@ -754,3 +760,8 @@ class TestSubpageBusinessRules(TestCase, WagtailTestUtils): # but we can add a BusinessChild to BusinessIndex response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.business_index.id))) self.assertEqual(response.status_code, 200) + + def test_not_prompted_for_page_type_when_only_one_choice(self): + response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.business_subindex.id, ))) + # BusinessChild is the only valid subpage type of BusinessSubIndex, so redirect straight there + self.assertRedirects(response, reverse('wagtailadmin_pages_create', args=('tests', 'businesschild', self.business_subindex.id))) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 559adb922..f12a08fe6 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -59,6 +59,12 @@ def add_subpage(request, parent_page_id): page_types = sorted(parent_page.clean_subpage_types(), key=lambda pagetype: pagetype.name.lower()) + if len(page_types) == 1: + # Only one page type is available - redirect straight to the create form rather than + # making the user choose + content_type = page_types[0] + return redirect('wagtailadmin_pages_create', content_type.app_label, content_type.model, parent_page.id) + return render(request, 'wagtailadmin/pages/add_subpage.html', { 'parent_page': parent_page, 'page_types': page_types, From c46ab6c20975528fd7b42e9bbb00a494606f5e4d Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 11:24:38 +0100 Subject: [PATCH 12/48] Renamed wagtailcore.util to wagtailcore.utils --- wagtail/wagtailadmin/edit_handlers.py | 2 +- wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py | 2 +- wagtail/wagtailcore/models.py | 2 +- wagtail/wagtailcore/util.py | 9 +++++---- wagtail/wagtailcore/utils.py | 6 ++++++ 5 files changed, 14 insertions(+), 7 deletions(-) create mode 100644 wagtail/wagtailcore/utils.py diff --git a/wagtail/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index 245f7ad60..9fd125f56 100644 --- a/wagtail/wagtailadmin/edit_handlers.py +++ b/wagtail/wagtailadmin/edit_handlers.py @@ -19,7 +19,7 @@ from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_lazy from wagtail.wagtailcore.models import Page -from wagtail.wagtailcore.util import camelcase_to_underscore +from wagtail.wagtailcore.utils import camelcase_to_underscore from wagtail.wagtailcore.fields import RichTextArea diff --git a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py index 2d85c355e..9f03c7b63 100644 --- a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py +++ b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py @@ -6,7 +6,7 @@ from wagtail.wagtailadmin import hooks from wagtail.wagtailadmin.menu import MenuItem from wagtail.wagtailcore.models import get_navigation_menu_items, UserPagePermissionsProxy -from wagtail.wagtailcore.util import camelcase_to_underscore +from wagtail.wagtailcore.utils import camelcase_to_underscore register = template.Library() diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 5c3f9d1c5..b14181faf 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -18,7 +18,7 @@ from django.utils.translation import ugettext_lazy as _ from treebeard.mp_tree import MP_Node -from wagtail.wagtailcore.util import camelcase_to_underscore +from wagtail.wagtailcore.utils import camelcase_to_underscore from wagtail.wagtailcore.query import PageQuerySet from wagtail.wagtailsearch import Indexed, get_search_backend diff --git a/wagtail/wagtailcore/util.py b/wagtail/wagtailcore/util.py index c5ad7ea8d..842fe24fa 100644 --- a/wagtail/wagtailcore/util.py +++ b/wagtail/wagtailcore/util.py @@ -1,6 +1,7 @@ -import re +import warnings +warnings.warn( + "The wagtail.wagtailcore.util module has been renamed. " + "Use wagtail.wagtailcore.utils instead.", DeprecationWarning) -def camelcase_to_underscore(str): - # http://djangosnippets.org/snippets/585/ - return re.sub('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))', '_\\1', str).lower().strip('_') +from .utils import * diff --git a/wagtail/wagtailcore/utils.py b/wagtail/wagtailcore/utils.py new file mode 100644 index 000000000..c5ad7ea8d --- /dev/null +++ b/wagtail/wagtailcore/utils.py @@ -0,0 +1,6 @@ +import re + + +def camelcase_to_underscore(str): + # http://djangosnippets.org/snippets/585/ + return re.sub('(((?<=[a-z])[A-Z])|([A-Z](?![A-Z]|$)))', '_\\1', str).lower().strip('_') From 46fc45877cf479f57bd105afa485133108bc24a2 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 11:27:34 +0100 Subject: [PATCH 13/48] Replaced get_query_set methods with get_queryset --- wagtail/wagtailcore/models.py | 38 +++++++++++++++++------------------ 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index b14181faf..1f0c98d83 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -126,56 +126,56 @@ def get_navigable_page_content_type_ids(): class PageManager(models.Manager): - def get_query_set(self): + def get_queryset(self): return PageQuerySet(self.model).order_by('path') def live(self): - return self.get_query_set().live() + return self.get_queryset().live() def not_live(self): - return self.get_query_set().not_live() + return self.get_queryset().not_live() def page(self, other): - return self.get_query_set().page(other) + return self.get_queryset().page(other) def not_page(self, other): - return self.get_query_set().not_page(other) + return self.get_queryset().not_page(other) def descendant_of(self, other, inclusive=False): - return self.get_query_set().descendant_of(other, inclusive) + return self.get_queryset().descendant_of(other, inclusive) def not_descendant_of(self, other, inclusive=False): - return self.get_query_set().not_descendant_of(other, inclusive) + return self.get_queryset().not_descendant_of(other, inclusive) def child_of(self, other): - return self.get_query_set().child_of(other) + return self.get_queryset().child_of(other) def not_child_of(self, other): - return self.get_query_set().not_child_of(other) + return self.get_queryset().not_child_of(other) def ancestor_of(self, other, inclusive=False): - return self.get_query_set().ancestor_of(other, inclusive) + return self.get_queryset().ancestor_of(other, inclusive) def not_ancestor_of(self, other, inclusive=False): - return self.get_query_set().not_ancestor_of(other, inclusive) + return self.get_queryset().not_ancestor_of(other, inclusive) def parent_of(self, other): - return self.get_query_set().parent_of(other) + return self.get_queryset().parent_of(other) def not_parent_of(self, other): - return self.get_query_set().not_parent_of(other) + return self.get_queryset().not_parent_of(other) def sibling_of(self, other, inclusive=False): - return self.get_query_set().sibling_of(other, inclusive) + return self.get_queryset().sibling_of(other, inclusive) def not_sibling_of(self, other, inclusive=False): - return self.get_query_set().not_sibling_of(other, inclusive) + return self.get_queryset().not_sibling_of(other, inclusive) def type(self, model): - return self.get_query_set().type(model) + return self.get_queryset().type(model) def not_type(self, model): - return self.get_query_set().not_type(model) + return self.get_queryset().not_type(model) class PageBase(models.base.ModelBase): @@ -697,8 +697,8 @@ class Orderable(models.Model): class SubmittedRevisionsManager(models.Manager): - def get_query_set(self): - return super(SubmittedRevisionsManager, self).get_query_set().filter(submitted_for_moderation=True) + def get_queryset(self): + return super(SubmittedRevisionsManager, self).get_queryset().filter(submitted_for_moderation=True) class PageRevision(models.Model): From 1ca6de4d463e7ed62bf72bc758c07b4522a13bce Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 11:54:12 +0100 Subject: [PATCH 14/48] Improvements to wagtaildocs pagination tests --- wagtail/wagtaildocs/tests.py | 86 ++++++++++++++++++++++++++++++++---- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtaildocs/tests.py b/wagtail/wagtaildocs/tests.py index 3353ce39e..576a3116f 100644 --- a/wagtail/wagtaildocs/tests.py +++ b/wagtail/wagtaildocs/tests.py @@ -56,11 +56,46 @@ class TestDocumentIndexView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertEqual(response.context['query_string'], "Hello") + def make_docs(self): + for i in range(50): + document = models.Document(title="Test " + str(i)) + document.save() + def test_pagination(self): - pages = ['0', '1', '-1', '9999', 'Not a page'] - for page in pages: - response = self.get({'p': page}) - self.assertEqual(response.status_code, 200) + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_index'), {'p': 2}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html') + + # Check that we got the correct page + self.assertEqual(response.context['documents'].number, 2) + + def test_pagination_invalid(self): + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_index'), {'p': 'Hello World!'}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html') + + # Check that we got page one + self.assertEqual(response.context['documents'].number, 1) + + def test_pagination_out_of_range(self): + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_index'), {'p': 99999}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html') + + # Check that we got the last page + self.assertEqual(response.context['documents'].number, response.context['documents'].paginator.num_pages) def test_ordering(self): orderings = ['title', '-created_at'] @@ -138,11 +173,46 @@ class TestDocumentChooserView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertEqual(response.context['query_string'], "Hello") + def make_docs(self): + for i in range(50): + document = models.Document(title="Test " + str(i)) + document.save() + def test_pagination(self): - pages = ['0', '1', '-1', '9999', 'Not a page'] - for page in pages: - response = self.get({'p': page}) - self.assertEqual(response.status_code, 200) + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_chooser'), {'p': 2}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/list.html') + + # Check that we got the correct page + self.assertEqual(response.context['documents'].number, 2) + + def test_pagination_invalid(self): + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_chooser'), {'p': 'Hello World!'}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/list.html') + + # Check that we got page one + self.assertEqual(response.context['documents'].number, 1) + + def test_pagination_out_of_range(self): + self.make_docs() + + response = self.client.get(reverse('wagtaildocs_chooser'), {'p': 99999}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtaildocs/documents/list.html') + + # Check that we got the last page + self.assertEqual(response.context['documents'].number, response.context['documents'].paginator.num_pages) class TestDocumentChooserChosenView(TestCase, WagtailTestUtils): From bf84b7f64a0903200f82a5b2ff562d6c639289aa Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 11:57:37 +0100 Subject: [PATCH 15/48] Removed get method from wagtaildocs tests --- wagtail/wagtaildocs/tests.py | 41 +++++++++--------------------------- 1 file changed, 10 insertions(+), 31 deletions(-) diff --git a/wagtail/wagtaildocs/tests.py b/wagtail/wagtaildocs/tests.py index 576a3116f..a39796c1d 100644 --- a/wagtail/wagtaildocs/tests.py +++ b/wagtail/wagtaildocs/tests.py @@ -43,16 +43,13 @@ class TestDocumentIndexView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_index'), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_index')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html') def test_search(self): - response = self.get({'q': "Hello"}) + response = self.client.get(reverse('wagtaildocs_index'), {'q': "Hello"}) self.assertEqual(response.status_code, 200) self.assertEqual(response.context['query_string'], "Hello") @@ -100,7 +97,7 @@ class TestDocumentIndexView(TestCase, WagtailTestUtils): def test_ordering(self): orderings = ['title', '-created_at'] for ordering in orderings: - response = self.get({'ordering': ordering}) + response = self.client.get(reverse('wagtaildocs_index'), {'ordering': ordering}) self.assertEqual(response.status_code, 200) @@ -108,11 +105,8 @@ class TestDocumentAddView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_add_document'), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_add_document')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/add.html') @@ -126,11 +120,8 @@ class TestDocumentEditView(TestCase, WagtailTestUtils): # Create a document to edit self.document = models.Document.objects.create(title="Test document") - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_edit_document', args=(self.document.id,)), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_edit_document', args=(self.document.id,))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/edit.html') @@ -144,11 +135,8 @@ class TestDocumentDeleteView(TestCase, WagtailTestUtils): # Create a document to delete self.document = models.Document.objects.create(title="Test document") - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_delete_document', args=(self.document.id,)), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_delete_document', args=(self.document.id,))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/confirm_delete.html') @@ -159,17 +147,14 @@ class TestDocumentChooserView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_chooser'), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_chooser')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.html') self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.js') def test_search(self): - response = self.get({'q': "Hello"}) + response = self.client.get(reverse('wagtaildocs_chooser'), {'q': "Hello"}) self.assertEqual(response.status_code, 200) self.assertEqual(response.context['query_string'], "Hello") @@ -222,11 +207,8 @@ class TestDocumentChooserChosenView(TestCase, WagtailTestUtils): # Create a document to choose self.document = models.Document.objects.create(title="Test document") - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_document_chosen', args=(self.document.id,)), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_document_chosen', args=(self.document.id,))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/chooser/document_chosen.js') @@ -237,11 +219,8 @@ class TestDocumentChooserUploadView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_chooser_upload'), params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtaildocs_chooser_upload')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.html') self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.js') From 929aa0f5e8725f7bd678dcb6a15b87de481a3e05 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 12:26:15 +0100 Subject: [PATCH 16/48] Added posting tests to wagtaildocs --- wagtail/wagtaildocs/tests.py | 76 ++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 7 deletions(-) diff --git a/wagtail/wagtaildocs/tests.py b/wagtail/wagtaildocs/tests.py index a39796c1d..a8420406b 100644 --- a/wagtail/wagtaildocs/tests.py +++ b/wagtail/wagtaildocs/tests.py @@ -110,22 +110,58 @@ class TestDocumentAddView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/add.html') - # TODO: Test posting + def test_post(self): + # Build a fake file + fake_file = ContentFile("A boring example document") + fake_file.name = 'test.txt' + + # Submit + post_data = { + 'title': "Test document", + 'file': fake_file, + } + response = self.client.post(reverse('wagtaildocs_add_document'), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtaildocs_index')) + + # Document should be created + self.assertTrue(models.Document.objects.filter(title="Test document").exists()) class TestDocumentEditView(TestCase, WagtailTestUtils): def setUp(self): self.login() + # Build a fake file + fake_file = ContentFile("A boring example document") + fake_file.name = 'test.txt' + # Create a document to edit - self.document = models.Document.objects.create(title="Test document") + self.document = models.Document.objects.create(title="Test document", file=fake_file) def test_simple(self): response = self.client.get(reverse('wagtaildocs_edit_document', args=(self.document.id,))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/edit.html') - # TODO: Test posting + def test_post(self): + # Build a fake file + fake_file = ContentFile("A boring example document") + fake_file.name = 'test.txt' + + # Submit title change + post_data = { + 'title': "Test document changed!", + 'file': fake_file, + } + response = self.client.post(reverse('wagtaildocs_edit_document', args=(self.document.id,)), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtaildocs_index')) + + # Document title should be changed + self.assertEqual(models.Document.objects.get(id=self.document.id).title, "Test document changed!") class TestDocumentDeleteView(TestCase, WagtailTestUtils): @@ -140,7 +176,18 @@ class TestDocumentDeleteView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/documents/confirm_delete.html') - # TODO: Test posting + def test_delete(self): + # Submit title change + post_data = { + 'foo': 'bar' + } + response = self.client.post(reverse('wagtaildocs_delete_document', args=(self.document.id,)), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtaildocs_index')) + + # Document should be deleted + self.assertFalse(models.Document.objects.filter(id=self.document.id).exists()) class TestDocumentChooserView(TestCase, WagtailTestUtils): @@ -212,8 +259,6 @@ class TestDocumentChooserChosenView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtaildocs/chooser/document_chosen.js') - # TODO: Test posting - class TestDocumentChooserUploadView(TestCase, WagtailTestUtils): def setUp(self): @@ -225,7 +270,24 @@ class TestDocumentChooserUploadView(TestCase, WagtailTestUtils): self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.html') self.assertTemplateUsed(response, 'wagtaildocs/chooser/chooser.js') - # TODO: Test document upload with chooser + def test_post(self): + # Build a fake file + fake_file = ContentFile("A boring example document") + fake_file.name = 'test.txt' + + # Submit + post_data = { + 'title': "Test document", + 'file': fake_file, + } + response = self.client.post(reverse('wagtaildocs_chooser_upload'), post_data) + + # Check that the response is a javascript file saying the document was chosen + self.assertTemplateUsed(response, 'wagtaildocs/chooser/document_chosen.js') + self.assertContains(response, "modal.respond('documentChosen'") + + # Document should be created + self.assertTrue(models.Document.objects.filter(title="Test document").exists()) class TestDocumentFilenameProperties(TestCase): From ec46d09772a5ecc99e8352f65f73f984e764a501 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 13:05:33 +0100 Subject: [PATCH 17/48] Improved editors picks pagination tests --- .../wagtailsearch/tests/test_editorspicks.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/wagtail/wagtailsearch/tests/test_editorspicks.py b/wagtail/wagtailsearch/tests/test_editorspicks.py index 7ee974fd0..ed37d9c62 100644 --- a/wagtail/wagtailsearch/tests/test_editorspicks.py +++ b/wagtail/wagtailsearch/tests/test_editorspicks.py @@ -1,4 +1,5 @@ from django.test import TestCase +from django.core.urlresolvers import reverse from wagtail.tests.utils import unittest, WagtailTestUtils from wagtail.wagtailsearch import models @@ -68,6 +69,51 @@ class TestEditorsPicksIndexView(TestCase, WagtailTestUtils): response = self.get({'p': page}) self.assertEqual(response.status_code, 200) + def make_editors_picks(self): + for i in range(50): + models.EditorsPick.objects.create( + query=models.Query.get("query " + str(i)), + page_id=1, + sort_order=0, + description="First editors pick", + ) + + def test_pagination(self): + self.make_editors_picks() + + response = self.client.get(reverse('wagtailsearch_editorspicks_index'), {'p': 2}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/index.html') + + # Check that we got the correct page + self.assertEqual(response.context['queries'].number, 2) + + def test_pagination_invalid(self): + self.make_editors_picks() + + response = self.client.get(reverse('wagtailsearch_editorspicks_index'), {'p': 'Hello World!'}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/index.html') + + # Check that we got page one + self.assertEqual(response.context['queries'].number, 1) + + def test_pagination_out_of_range(self): + self.make_editors_picks() + + response = self.client.get(reverse('wagtailsearch_editorspicks_index'), {'p': 99999}) + + # Check response + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/index.html') + + # Check that we got the last page + self.assertEqual(response.context['queries'].number, response.context['queries'].paginator.num_pages) + class TestEditorsPicksAddView(TestCase, WagtailTestUtils): def setUp(self): From 620e58632f374c464828f7e75a38b66b81517c6a Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 13:08:54 +0100 Subject: [PATCH 18/48] General improvements to editorspicks tests --- .../wagtailsearch/tests/test_editorspicks.py | 29 ++++--------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/wagtail/wagtailsearch/tests/test_editorspicks.py b/wagtail/wagtailsearch/tests/test_editorspicks.py index ed37d9c62..c92dd4ef6 100644 --- a/wagtail/wagtailsearch/tests/test_editorspicks.py +++ b/wagtail/wagtailsearch/tests/test_editorspicks.py @@ -1,5 +1,6 @@ from django.test import TestCase from django.core.urlresolvers import reverse + from wagtail.tests.utils import unittest, WagtailTestUtils from wagtail.wagtailsearch import models @@ -50,25 +51,16 @@ class TestEditorsPicksIndexView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/', params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtailsearch_editorspicks_index')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/index.html') def test_search(self): - response = self.get({'q': "Hello"}) + response = self.client.get(reverse('wagtailsearch_editorspicks_index'), {'q': "Hello"}) self.assertEqual(response.status_code, 200) self.assertEqual(response.context['query_string'], "Hello") - def test_pagination(self): - pages = ['0', '1', '-1', '9999', 'Not a page'] - for page in pages: - response = self.get({'p': page}) - self.assertEqual(response.status_code, 200) - def make_editors_picks(self): for i in range(50): models.EditorsPick.objects.create( @@ -119,11 +111,8 @@ class TestEditorsPicksAddView(TestCase, WagtailTestUtils): def setUp(self): self.login() - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/add/', params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtailsearch_editorspicks_add')) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/add.html') @@ -138,11 +127,8 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): self.query = models.Query.get("Hello") self.query.editors_picks.create(page_id=1, description="Root page") - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/', params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/edit.html') @@ -157,11 +143,8 @@ class TestEditorsPicksDeleteView(TestCase, WagtailTestUtils): self.query = models.Query.get("Hello") self.query.editors_picks.create(page_id=1, description="Root page") - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/delete/', params) - def test_simple(self): - response = self.get() + response = self.client.get(reverse('wagtailsearch_editorspicks_delete', args=(self.query.id, ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/confirm_delete.html') From a05df1a534d1f6cfadd65c7fdbcaf2f799e12e70 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 13:39:12 +0100 Subject: [PATCH 19/48] Added posting tests to editors picks --- .../wagtailsearch/tests/test_editorspicks.py | 135 +++++++++++++++++- 1 file changed, 130 insertions(+), 5 deletions(-) diff --git a/wagtail/wagtailsearch/tests/test_editorspicks.py b/wagtail/wagtailsearch/tests/test_editorspicks.py index c92dd4ef6..211a54cc9 100644 --- a/wagtail/wagtailsearch/tests/test_editorspicks.py +++ b/wagtail/wagtailsearch/tests/test_editorspicks.py @@ -116,7 +116,39 @@ class TestEditorsPicksAddView(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/add.html') - # TODO: Test posting + def test_post(self): + # Submit + post_data = { + 'query_string': "test", + 'editors_picks-TOTAL_FORMS': 1, + 'editors_picks-INITIAL_FORMS': 0, + 'editors_picks-MAX_NUM_FORMS': 1000, + 'editors_picks-0-DELETE': '', + 'editors_picks-0-ORDER': 0, + 'editors_picks-0-page': 1, + 'editors_picks-0-description': "Hello", + } + response = self.client.post(reverse('wagtailsearch_editorspicks_add'), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtailsearch_editorspicks_index')) + + # Check that the editors pick was created + self.assertTrue(models.Query.get('test').editors_picks.filter(page_id=1).exists()) + + def test_post_without_recommendations(self): + # Submit + post_data = { + 'query_string': "test", + 'editors_picks-TOTAL_FORMS': 0, + 'editors_picks-INITIAL_FORMS': 0, + 'editors_picks-MAX_NUM_FORMS': 1000, + } + response = self.client.post(reverse('wagtailsearch_editorspicks_add'), post_data) + + # User should be given an error + self.assertEqual(response.status_code, 200) + self.assertFormsetError(response, 'editors_pick_formset', None, None, "Please specify at least one recommendation for this search term.") class TestEditorsPicksEditView(TestCase, WagtailTestUtils): @@ -125,14 +157,92 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): # Create an editors pick to edit self.query = models.Query.get("Hello") - self.query.editors_picks.create(page_id=1, description="Root page") + self.editors_pick = self.query.editors_picks.create(page_id=1, description="Root page") + self.editors_pick_2 = self.query.editors_picks.create(page_id=2, description="Homepage") def test_simple(self): response = self.client.get(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/edit.html') - # TODO: Test posting + def test_post(self): + # Submit + post_data = { + 'query_string': "Hello", + 'editors_picks-TOTAL_FORMS': 2, + 'editors_picks-INITIAL_FORMS': 2, + 'editors_picks-MAX_NUM_FORMS': 1000, + 'editors_picks-0-id': self.editors_pick.id, + 'editors_picks-0-DELETE': '', + 'editors_picks-0-ORDER': 0, + 'editors_picks-0-page': 1, + 'editors_picks-0-description': "Description has changed", # Change + 'editors_picks-1-id': self.editors_pick_2.id, + 'editors_picks-1-DELETE': '', + 'editors_picks-1-ORDER': 0, + 'editors_picks-1-page': 2, + 'editors_picks-1-description': "Homepage", + } + response = self.client.post(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, )), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtailsearch_editorspicks_index')) + + # Check that the editors pick description was edited + self.assertEqual(models.EditorsPick.objects.get(id=self.editors_pick.id).description, "Description has changed") + + def test_post_delete_recommendation(self): + # Submit + post_data = { + 'query_string': "Hello", + 'editors_picks-TOTAL_FORMS': 2, + 'editors_picks-INITIAL_FORMS': 2, + 'editors_picks-MAX_NUM_FORMS': 1000, + 'editors_picks-0-id': self.editors_pick.id, + 'editors_picks-0-DELETE': '', + 'editors_picks-0-ORDER': 0, + 'editors_picks-0-page': 1, + 'editors_picks-0-description': "Root page", + 'editors_picks-1-id': self.editors_pick_2.id, + 'editors_picks-1-DELETE': 1, + 'editors_picks-1-ORDER': 0, + 'editors_picks-1-page': 2, + 'editors_picks-1-description': "Homepage", + } + response = self.client.post(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, )), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtailsearch_editorspicks_index')) + + # Check that the recommendation was deleted + self.assertFalse(models.EditorsPick.objects.filter(id=self.editors_pick_2.id).exists()) + + # The other recommendation should still exist + self.assertTrue(models.EditorsPick.objects.filter(id=self.editors_pick.id).exists()) + + def test_post_without_recommendations(self): + # Submit + post_data = { + 'query_string': "Hello", + 'editors_picks-TOTAL_FORMS': 2, + 'editors_picks-INITIAL_FORMS': 2, + 'editors_picks-MAX_NUM_FORMS': 1000, + 'editors_picks-0-id': self.editors_pick.id, + 'editors_picks-0-DELETE': 1, + 'editors_picks-0-ORDER': 0, + 'editors_picks-0-page': 1, + 'editors_picks-0-description': "Description has changed", # Change + 'editors_picks-1-id': self.editors_pick_2.id, + 'editors_picks-1-DELETE': 1, + 'editors_picks-1-ORDER': 0, + 'editors_picks-1-page': 2, + 'editors_picks-1-description': "Homepage", + } + response = self.client.post(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, )), post_data) + + # User should be given an error + self.assertEqual(response.status_code, 200) + self.assertFormsetError(response, 'editors_pick_formset', None, None, "Please specify at least one recommendation for this search term.") class TestEditorsPicksDeleteView(TestCase, WagtailTestUtils): @@ -141,11 +251,26 @@ class TestEditorsPicksDeleteView(TestCase, WagtailTestUtils): # Create an editors pick to delete self.query = models.Query.get("Hello") - self.query.editors_picks.create(page_id=1, description="Root page") + self.editors_pick = self.query.editors_picks.create(page_id=1, description="Root page") + self.editors_pick_2 = self.query.editors_picks.create(page_id=2, description="Homepage") def test_simple(self): response = self.client.get(reverse('wagtailsearch_editorspicks_delete', args=(self.query.id, ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/confirm_delete.html') - # TODO: Test posting + def test_post(self): + # Submit + post_data = { + 'foo': 'bar', + } + response = self.client.post(reverse('wagtailsearch_editorspicks_delete', args=(self.query.id, )), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtailsearch_editorspicks_index')) + + # Check that both recommendations were deleted + self.assertFalse(models.EditorsPick.objects.filter(id=self.editors_pick_2.id).exists()) + + # The other recommendation should still exist + self.assertFalse(models.EditorsPick.objects.filter(id=self.editors_pick.id).exists()) From e220c77cc01727650959173e47981755d4540e25 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 14:20:42 +0100 Subject: [PATCH 20/48] Added test to test reordering of editors picks --- .../wagtailsearch/tests/test_editorspicks.py | 37 +++++++++++++++++-- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailsearch/tests/test_editorspicks.py b/wagtail/wagtailsearch/tests/test_editorspicks.py index 211a54cc9..99debab40 100644 --- a/wagtail/wagtailsearch/tests/test_editorspicks.py +++ b/wagtail/wagtailsearch/tests/test_editorspicks.py @@ -179,7 +179,7 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): 'editors_picks-0-description': "Description has changed", # Change 'editors_picks-1-id': self.editors_pick_2.id, 'editors_picks-1-DELETE': '', - 'editors_picks-1-ORDER': 0, + 'editors_picks-1-ORDER': 1, 'editors_picks-1-page': 2, 'editors_picks-1-description': "Homepage", } @@ -191,6 +191,37 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): # Check that the editors pick description was edited self.assertEqual(models.EditorsPick.objects.get(id=self.editors_pick.id).description, "Description has changed") + def test_post_reorder(self): + # Check order before reordering + self.assertEqual(models.Query.get("Hello").editors_picks.all()[0], self.editors_pick) + self.assertEqual(models.Query.get("Hello").editors_picks.all()[1], self.editors_pick_2) + + # Submit + post_data = { + 'query_string': "Hello", + 'editors_picks-TOTAL_FORMS': 2, + 'editors_picks-INITIAL_FORMS': 2, + 'editors_picks-MAX_NUM_FORMS': 1000, + 'editors_picks-0-id': self.editors_pick.id, + 'editors_picks-0-DELETE': '', + 'editors_picks-0-ORDER': 1, # Change + 'editors_picks-0-page': 1, + 'editors_picks-0-description': "Root page", + 'editors_picks-1-id': self.editors_pick_2.id, + 'editors_picks-1-DELETE': '', + 'editors_picks-1-ORDER': 0, # Change + 'editors_picks-1-page': 2, + 'editors_picks-1-description': "Homepage", + } + response = self.client.post(reverse('wagtailsearch_editorspicks_edit', args=(self.query.id, )), post_data) + + # User should be redirected back to the index + self.assertRedirects(response, reverse('wagtailsearch_editorspicks_index')) + + # Check that the recommendations were reordered + self.assertEqual(models.Query.get("Hello").editors_picks.all()[0], self.editors_pick_2) + self.assertEqual(models.Query.get("Hello").editors_picks.all()[1], self.editors_pick) + def test_post_delete_recommendation(self): # Submit post_data = { @@ -205,7 +236,7 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): 'editors_picks-0-description': "Root page", 'editors_picks-1-id': self.editors_pick_2.id, 'editors_picks-1-DELETE': 1, - 'editors_picks-1-ORDER': 0, + 'editors_picks-1-ORDER': 1, 'editors_picks-1-page': 2, 'editors_picks-1-description': "Homepage", } @@ -234,7 +265,7 @@ class TestEditorsPicksEditView(TestCase, WagtailTestUtils): 'editors_picks-0-description': "Description has changed", # Change 'editors_picks-1-id': self.editors_pick_2.id, 'editors_picks-1-DELETE': 1, - 'editors_picks-1-ORDER': 0, + 'editors_picks-1-ORDER': 1, 'editors_picks-1-page': 2, 'editors_picks-1-description': "Homepage", } From cbc401befe360490c49add0dfc86454cb3bf0543 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 14:50:42 +0100 Subject: [PATCH 21/48] Deleted test to test query popularity over time (not implemented) --- wagtail/wagtailsearch/tests/test_queries.py | 39 --------------------- 1 file changed, 39 deletions(-) diff --git a/wagtail/wagtailsearch/tests/test_queries.py b/wagtail/wagtailsearch/tests/test_queries.py index 5c341d7d6..70854c785 100644 --- a/wagtail/wagtailsearch/tests/test_queries.py +++ b/wagtail/wagtailsearch/tests/test_queries.py @@ -102,45 +102,6 @@ class TestQueryPopularity(TestCase): self.assertEqual(popular_queries[1], models.Query.get("popular query")) self.assertEqual(popular_queries[2], models.Query.get("little popular query")) - @unittest.expectedFailure # Time based popularity isn't implemented yet - def test_query_popularity_over_time(self): - today = timezone.now().date() - two_days_ago = today - datetime.timedelta(days=2) - a_week_ago = today - datetime.timedelta(days=7) - a_month_ago = today - datetime.timedelta(days=30) - - # Add 10 hits to a query that was very popular query a month ago - for i in range(10): - models.Query.get("old popular query").add_hit(date=a_month_ago) - - # Add 5 hits to a query that is was popular 2 days ago - for i in range(5): - models.Query.get("new popular query").add_hit(date=two_days_ago) - - # Get most popular queries - popular_queries = models.Query.get_most_popular() - - # Old popular query should be most popular - self.assertEqual(popular_queries.count(), 2) - self.assertEqual(popular_queries[0], models.Query.get("old popular query")) - self.assertEqual(popular_queries[1], models.Query.get("new popular query")) - - # Get most popular queries for past week - past_week_popular_queries = models.Query.get_most_popular(date_since=a_week_ago) - - # Only new popular query should be in this list - self.assertEqual(past_week_popular_queries.count(), 1) - self.assertEqual(past_week_popular_queries[0], models.Query.get("new popular query")) - - # Old popular query gets a couple more hits! - for i in range(2): - models.Query.get("old popular query").add_hit() - - # Old popular query should now be in the most popular queries - self.assertEqual(past_week_popular_queries.count(), 2) - self.assertEqual(past_week_popular_queries[0], models.Query.get("new popular query")) - self.assertEqual(past_week_popular_queries[1], models.Query.get("old popular query")) - class TestGarbageCollectCommand(TestCase): def test_garbage_collect_command(self): From ed4af844dec210792e5e9dd474b7d7211769f3d2 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 15:58:54 +0100 Subject: [PATCH 22/48] Added tests for userbar --- wagtail/wagtailadmin/tests/test_userbar.py | 82 ++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 wagtail/wagtailadmin/tests/test_userbar.py diff --git a/wagtail/wagtailadmin/tests/test_userbar.py b/wagtail/wagtailadmin/tests/test_userbar.py new file mode 100644 index 000000000..52ec2d60a --- /dev/null +++ b/wagtail/wagtailadmin/tests/test_userbar.py @@ -0,0 +1,82 @@ +from django.test import TestCase +from django.test.client import RequestFactory +from django.core.urlresolvers import reverse +from django.template import Template, Context +from django.contrib.auth.models import User, AnonymousUser + +from wagtail.tests.utils import WagtailTestUtils +from wagtail.wagtailcore.models import Page + + +class TestUserbarTag(TestCase): + def setUp(self): + self.user = User.objects.create_superuser(username='test', email='test@email.com', password='password') + self.homepage = Page.objects.get(id=2) + + def dummy_request(self, user=None): + request = RequestFactory().get('/') + request.user = user or AnonymousUser() + return request + + def test_userbar_tag(self): + template = Template("{% load wagtailuserbar %}{% wagtailuserbar %}") + content = template.render(Context({ + 'self': self.homepage, + 'request': self.dummy_request(self.user), + })) + + self.assertIn("", content) + + def test_userbar_tag_anonymous_user(self): + template = Template("{% load wagtailuserbar %}{% wagtailuserbar %}") + content = template.render(Context({ + 'self': self.homepage, + 'request': self.dummy_request(), + })) + + # Make sure nothing was rendered + self.assertEqual(content, '') + + +class TestUserbarFrontend(TestCase, WagtailTestUtils): + def setUp(self): + self.login() + self.homepage = Page.objects.get(id=2) + + def test_userbar_frontend(self): + response = self.client.get(reverse('wagtailadmin_userbar_frontend', args=(self.homepage.id, ))) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/userbar/base.html') + + def test_userbar_anonymous_user_cannot_see(self): + # Logout + self.client.logout() + + response = self.client.get(reverse('wagtailadmin_userbar_frontend', args=(self.homepage.id, ))) + + # Check that the user recieved a forbidden message + self.assertEqual(response.status_code, 403) + + +class TestUserbarModeration(TestCase, WagtailTestUtils): + def setUp(self): + self.login() + self.homepage = Page.objects.get(id=2) + self.homepage.save_revision() + self.revision = self.homepage.get_latest_revision() + + def test_userbar_frontend(self): + response = self.client.get(reverse('wagtailadmin_userbar_moderation', args=(self.revision.id, ))) + + self.assertEqual(response.status_code, 200) + self.assertTemplateUsed(response, 'wagtailadmin/userbar/base.html') + + def test_userbar_anonymous_user_cannot_see(self): + # Logout + self.client.logout() + + response = self.client.get(reverse('wagtailadmin_userbar_moderation', args=(self.revision.id, ))) + + # Check that the user recieved a forbidden message + self.assertEqual(response.status_code, 403) From 06dc598d0a928109aee35517f6bd161bcaf64ba8 Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Wed, 18 Jun 2014 17:25:58 +0100 Subject: [PATCH 23/48] Update CHANGELOG.txt --- CHANGELOG.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 039935b43..6686cf973 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -9,8 +9,8 @@ Changelog * Removed the "More" section from the admin menu * Added pagination to page listings in admin * Support for setting a subpage_types property on page models, to define which page types are allowed as subpages - * Added a new datetime picker - * Added styleguide + * Added a new datetime picker widget + * Added styleguide (mainly for wagtail developers) * Aesthetic improvements to preview experience * Fix: Animated GIFs are now coalesced before resizing * Fix: Wand backend clones images before modifying them From d95763251d5f70f819656fafeadcf3c7fb6c2603 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 18 Jun 2014 17:29:24 +0100 Subject: [PATCH 24/48] Corrected names of userbar tests --- wagtail/wagtailadmin/tests/test_userbar.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wagtail/wagtailadmin/tests/test_userbar.py b/wagtail/wagtailadmin/tests/test_userbar.py index 52ec2d60a..acdb81690 100644 --- a/wagtail/wagtailadmin/tests/test_userbar.py +++ b/wagtail/wagtailadmin/tests/test_userbar.py @@ -49,7 +49,7 @@ class TestUserbarFrontend(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/userbar/base.html') - def test_userbar_anonymous_user_cannot_see(self): + def test_userbar_frontend_anonymous_user_cannot_see(self): # Logout self.client.logout() @@ -66,13 +66,13 @@ class TestUserbarModeration(TestCase, WagtailTestUtils): self.homepage.save_revision() self.revision = self.homepage.get_latest_revision() - def test_userbar_frontend(self): + def test_userbar_moderation(self): response = self.client.get(reverse('wagtailadmin_userbar_moderation', args=(self.revision.id, ))) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'wagtailadmin/userbar/base.html') - def test_userbar_anonymous_user_cannot_see(self): + def test_userbar_moderation_anonymous_user_cannot_see(self): # Logout self.client.logout() From 03ce91c992c1c8d2dc49649d10b35add650fbd90 Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Thu, 19 Jun 2014 12:28:07 +0100 Subject: [PATCH 25/48] Update CONTRIBUTORS.rst --- CONTRIBUTORS.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTORS.rst b/CONTRIBUTORS.rst index a3f9c7221..fcba8d2d7 100644 --- a/CONTRIBUTORS.rst +++ b/CONTRIBUTORS.rst @@ -1,8 +1,8 @@ Original Authors ================ -* Matthew Westcott matthew.westcott@torchbox.com -* David Cranwell david.cranwell@torchbox.com +* Matthew Westcott matthew.westcott@torchbox.com twitter: @gasmanic +* David Cranwell david.cranwell@torchbox.com twitter: @davecranwell * Karl Hobley karl.hobley@torchbox.com * Helen Chapman helen.chapman@torchbox.com From 4c5a1b1ae9a16821bdade3fd133eaca2b1e82d18 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Thu, 19 Jun 2014 14:17:27 +0100 Subject: [PATCH 26/48] Recache coverage badge --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 62b3c5cd5..0e68b8c6b 100644 --- a/README.rst +++ b/README.rst @@ -1,7 +1,7 @@ .. image:: https://travis-ci.org/torchbox/wagtail.png?branch=master :target: https://travis-ci.org/torchbox/wagtail -.. image:: https://coveralls.io/repos/torchbox/wagtail/badge.png?branch=master&zxcv +.. image:: https://coveralls.io/repos/torchbox/wagtail/badge.png?branch=master&zxcv1 :target: https://coveralls.io/r/torchbox/wagtail?branch=master .. image:: https://pypip.in/v/wagtail/badge.png?zxcv From 4e8e1f97ca250920f958ffdd732edbf4de816402 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 17:28:34 +0100 Subject: [PATCH 27/48] use self.attrs in AbstractRendition.img_tag to avoid duplication --- wagtail/wagtailimages/models.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 17e1ece86..8535199e7 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -243,9 +243,7 @@ class AbstractRendition(models.Model): ) def img_tag(self): - return mark_safe( - '%s' % (escape(self.url), self.width, self.height, escape(self.image.title)) - ) + return mark_safe('' % self.attrs) class Meta: abstract = True From f6ae1834fa350692db2632be6f6714b69726c4e6 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 17:34:59 +0100 Subject: [PATCH 28/48] Document the 'attrs' property of image renditions --- docs/building_your_site/frontenddevelopers.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index f0054eaab..a8cf6a25e 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -197,6 +197,11 @@ In some cases greater control over the ``img`` tag is required, for example to a {{ tmp_photo.alt }} +You can also use the ``attrs`` property as a shorthand to output the ``src``, ``width``, ``height`` and ``alt`` attributes in one go: + +.. code-block:: django + + .. _rich-text-filter: From aad0e24a48812a683ad295870a700b07eee31eac Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 17:36:57 +0100 Subject: [PATCH 29/48] Changelog entry for #299 --- CHANGELOG.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 6686cf973..c577040c6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -12,6 +12,7 @@ Changelog * Added a new datetime picker widget * Added styleguide (mainly for wagtail developers) * Aesthetic improvements to preview experience + * Added an 'attrs' property to image rendition objects to output src, width, height and alt attributes all in one go * Fix: Animated GIFs are now coalesced before resizing * Fix: Wand backend clones images before modifying them * Fix: Admin breadcrumb now positioned correctly on mobile From 5e055972cde55f373a41e1012e1ada3a5b07e9b4 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 21:55:45 +0100 Subject: [PATCH 30/48] Refactor image tag parser so that it only has two cases to handle --- wagtail/wagtailimages/models.py | 4 +-- .../wagtailimages/templatetags/image_tags.py | 25 ++++++------------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 550be2034..3ff2f9782 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -68,8 +68,8 @@ class AbstractImage(models.Model, TagSearchable): except ObjectDoesNotExist: file_field = self.file - # If we have a backend attribute then pass it to process - # image - else pass 'default' + # If we have a backend attribute then pass it to process + # image - else pass 'default' backend_name = getattr(self, 'backend', 'default') generated_image_file = filter.process_image(file_field.file, backend_name=backend_name) diff --git a/wagtail/wagtailimages/templatetags/image_tags.py b/wagtail/wagtailimages/templatetags/image_tags.py index e8a4c2191..5c7273417 100644 --- a/wagtail/wagtailimages/templatetags/image_tags.py +++ b/wagtail/wagtailimages/templatetags/image_tags.py @@ -15,31 +15,22 @@ def image(parser, token): filter_spec = bits[1] bits = bits[2:] - if len(bits) == 0: - # token is of the form {% image self.photo max-320x200 %} - return ImageNode(image_var, filter_spec) - - elif len(bits) == 2: + if len(bits) == 2 and bits[0] == 'as': # token is of the form {% image self.photo max-320x200 as img %} - - if bits[0] == 'as': - return ImageNode(image_var, filter_spec, output_var_name=bits[1]) - - if len(bits) > 0: - # customized attrs + return ImageNode(image_var, filter_spec, output_var_name=bits[1]) + else: + # token is of the form {% image self.photo max-320x200 %} - all additional tokens + # should be kwargs, which become attributes attrs = {} for bit in bits: try: - name,value = bit.split('=') - except: - raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 [ custom-attr=\"value\" [ ... ] ] %%} or {%% image self.photo max-320x200 as img %%}") + name, value = bit.split('=') + except ValueError: + raise template.TemplateSyntaxError("'image' tag should be of the form {% image self.photo max-320x200 [ custom-attr=\"value\" ... ] %} or {% image self.photo max-320x200 as img %}") attrs[name] = parser.compile_filter(value) # setup to resolve context variables as value return ImageNode(image_var, filter_spec, attrs=attrs) - # something is wrong if we made it this far - raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 [ custom-attr=\"value\" [ ... ] ] %%} or {%% image self.photo max-320x200 as img %%}") - class ImageNode(template.Node): def __init__(self, image_var_name, filter_spec, output_var_name=None, attrs={}): From 0c4b5ba5566f1a32bc6fe1769741407a18820298 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 22:02:45 +0100 Subject: [PATCH 31/48] Add unit test for image tag with extra attributes --- wagtail/wagtailimages/tests.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/wagtail/wagtailimages/tests.py b/wagtail/wagtailimages/tests.py index d546c396d..2fbac118c 100644 --- a/wagtail/wagtailimages/tests.py +++ b/wagtail/wagtailimages/tests.py @@ -210,6 +210,20 @@ class TestImageTag(TestCase): self.assertTrue('height="300"' in result) self.assertTrue('alt="Test image"' in result) + def render_image_tag_with_extra_attributes(self, image, title): + temp = template.Template('{% load image_tags %}{% image image_obj width-400 class="photo" title=title|lower %}') + context = template.Context({'image_obj': image, 'title': title}) + return temp.render(context) + + def test_image_tag_with_extra_attributes(self): + result = self.render_image_tag_with_extra_attributes(self.image, 'My Wonderful Title') + + # Check that all the required HTML attributes are set + self.assertTrue('width="400"' in result) + self.assertTrue('height="300"' in result) + self.assertTrue('class="photo"' in result) + self.assertTrue('title="my wonderful title"' in result) + ## ===== ADMIN VIEWS ===== From b26744e13790b730e2fdc6214af261032c36adba Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Thu, 19 Jun 2014 22:06:41 +0100 Subject: [PATCH 32/48] changelog entry for #294 --- CHANGELOG.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index c577040c6..19eec21e7 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -12,6 +12,7 @@ Changelog * Added a new datetime picker widget * Added styleguide (mainly for wagtail developers) * Aesthetic improvements to preview experience + * 'image' tag now accepts extra keyword arguments to be output as attributes on the img tag * Added an 'attrs' property to image rendition objects to output src, width, height and alt attributes all in one go * Fix: Animated GIFs are now coalesced before resizing * Fix: Wand backend clones images before modifying them From 4081206c77fd5c9bcfc03dfe6ed923dfa4d93f23 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sat, 5 Apr 2014 20:10:58 +0100 Subject: [PATCH 33/48] Scrapped ElasticUtils, use Elasticsearch-py instead Conflicts: wagtail/wagtailsearch/backends/elasticsearch.py wagtail/wagtailsearch/indexed.py --- runtests.py | 2 +- .../wagtailsearch/backends/elasticsearch.py | 187 +++++++++++++----- 2 files changed, 142 insertions(+), 47 deletions(-) diff --git a/runtests.py b/runtests.py index 96653b40d..efa66910f 100755 --- a/runtests.py +++ b/runtests.py @@ -13,7 +13,7 @@ MEDIA_ROOT = os.path.join(WAGTAIL_ROOT, 'test-media') if not settings.configured: try: - import elasticutils + import elasticsearch has_elasticsearch = True except ImportError: has_elasticsearch = False diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index d2b58d564..7571db5ac 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -1,6 +1,8 @@ +from __future__ import absolute_import + from django.db import models -from elasticutils import get_es, S +from elasticsearch import Elasticsearch, NotFoundError from wagtail.wagtailsearch.backends.base import BaseSearch from wagtail.wagtailsearch.indexed import Indexed @@ -9,16 +11,131 @@ import string class ElasticSearchResults(object): - def __init__(self, model, query, prefetch_related=[]): + def __init__(self, backend, model, query_string, fields=None, filters={}, prefetch_related=[]): + self.backend = backend self.model = model - self.query = query - self.count = query.count() + self.query_string = query_string + self.fields = fields + self.filters = filters self.prefetch_related = prefetch_related + def _get_filters(self): + # Filters + filters = [] + + # Filter by content type + filters.append({ + 'prefix': { + 'content_type': self.model.indexed_get_content_type() + } + }) + + # Extra filters + if self.filters: + for key, value in self.filters.items(): + if '__' in key: + field, lookup = key.split('__') + else: + field = key + lookup = None + + if lookup is None: + if value is None: + filters.append({ + 'missing': { + 'field': field, + } + }) + else: + filters.append({ + 'term': { + field: value + } + }) + + if lookup in ['startswith', 'prefix']: + filters.append({ + 'prefix': { + field: value + } + }) + + if lookup in ['gt', 'gte', 'lt', 'lte']: + filters.append({ + 'range': { + field: { + lookup: value, + } + } + }) + + if lookup == 'range': + lower, upper = value + filters.append({ + 'range': { + field: { + 'gte': lower, + 'lte': upper, + } + } + }) + + return filters + + def _get_query(self): + # Query + query = { + 'query_string': { + 'query': self.query_string, + } + } + + # Fields + if self.fields: + query['query_string']['fields'] = self.fields + + # Filters + filters = self._get_filters() + + return { + 'query': { + 'filtered': { + 'query': query, + 'filter': { + 'and': filters, + } + } + } + } + + def _get_results_pks(self, offset=0, limit=None): + query = self._get_query() + query['query']['from'] = offset + if limit is not None: + query['query']['size'] = limit + + hits = self.backend.es.search( + index=self.backend.es_index, + body=query, + _source=False, + fields='pk', + ) + + return [hit['fields']['pk'][0] for hit in hits['hits']['hits']] + + def _get_count(self): + query = self._get_query() + count = self.backend.es.count( + index=self.backend.es_index, + body=query, + ) + + return count['count'] + def __getitem__(self, key): if isinstance(key, slice): # Get primary keys - pk_list_unclean = [result._source["pk"] for result in self.query[key]] + pk_list_unclean = self._get_results_pks(key.start, key.stop - key.start) # Remove duplicate keys (and preserve order) seen_pks = set() @@ -45,11 +162,11 @@ class ElasticSearchResults(object): return results_sorted else: # Return a single item - pk = self.query[key]._source["pk"] + pk = self._get_results_pks(key, key + 1)[0] return self.model.objects.get(pk=pk) def __len__(self): - return self.count + return self._get_count() class ElasticSearch(BaseSearch): @@ -64,22 +181,17 @@ class ElasticSearch(BaseSearch): # Get ElasticSearch interface # Any remaining params are passed into the ElasticSearch constructor - self.es = get_es( + self.es = Elasticsearch( urls=self.es_urls, timeout=self.es_timeout, force_new=self.es_force_new, **params) - self.s = S().es( - urls=self.es_urls, - timeout=self.es_timeout, - force_new=self.es_force_new, - **params).indexes(self.es_index) def reset_index(self): # Delete old index try: - self.es.delete_index(self.es_index) - except: + self.es.indices.delete(self.es_index) + except NotFoundError: pass # Settings @@ -128,7 +240,7 @@ class ElasticSearch(BaseSearch): } # Create new index - self.es.create_index(self.es_index, INDEX_SETTINGS) + self.es.indices.create(self.es_index, INDEX_SETTINGS) def add_type(self, model): # Get type name @@ -144,14 +256,14 @@ class ElasticSearch(BaseSearch): }.items() + indexed_fields.items()) # Put mapping - self.es.put_mapping(self.es_index, content_type, { + self.es.indices.put_mapping(index=self.es_index, doc_type=content_type, body={ content_type: { "properties": fields, } }) def refresh_index(self): - self.es.refresh(self.es_index) + self.es.indices.refresh(self.es_index) def add(self, obj): # Make sure the object can be indexed @@ -165,6 +277,10 @@ class ElasticSearch(BaseSearch): self.es.index(self.es_index, obj.indexed_get_content_type(), doc, id=doc["id"]) def add_bulk(self, obj_list): + # TODO: Make this work with new elastic search module + for obj in obj_list: + self.add(obj) + return # Group all objects by their type type_set = {} for obj in obj_list: @@ -194,13 +310,14 @@ class ElasticSearch(BaseSearch): if not isinstance(obj, Indexed) or not isinstance(obj, models.Model): return - # Get ID for document - doc_id = obj.indexed_get_document_id() - # Delete document try: - self.es.delete(self.es_index, obj.indexed_get_content_type(), doc_id) - except: + self.es.delete( + self.es_index, + obj.indexed_get_content_type(), + obj.indexed_get_document_id(), + ) + except NotFoundError: pass # Document doesn't exist, ignore this exception def search(self, query_string, model, fields=None, filters={}, prefetch_related=[]): @@ -215,27 +332,5 @@ class ElasticSearch(BaseSearch): if not query_string: return [] - # Query - if fields: - query = self.s.query_raw({ - "query_string": { - "query": query_string, - "fields": fields, - } - }) - else: - query = self.s.query_raw({ - "query_string": { - "query": query_string, - } - }) - - # Filter results by this content type - query = query.filter(content_type__prefix=model.indexed_get_content_type()) - - # Extra filters - if filters: - query = query.filter(**filters) - # Return search results - return ElasticSearchResults(model, query, prefetch_related=prefetch_related) + return ElasticSearchResults(self, model, query_string, fields=fields, filters=filters, prefetch_related=prefetch_related) From c00c4d24372079e8a07cc4276cfb2568643abd80 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 6 Apr 2014 14:35:57 +0100 Subject: [PATCH 34/48] New ElasticSearch module now supports bulk insert --- .../wagtailsearch/backends/elasticsearch.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 7571db5ac..78de0e94f 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -3,6 +3,7 @@ from __future__ import absolute_import from django.db import models from elasticsearch import Elasticsearch, NotFoundError +from elasticsearch.helpers import bulk from wagtail.wagtailsearch.backends.base import BaseSearch from wagtail.wagtailsearch.indexed import Indexed @@ -277,10 +278,6 @@ class ElasticSearch(BaseSearch): self.es.index(self.es_index, obj.indexed_get_content_type(), doc, id=doc["id"]) def add_bulk(self, obj_list): - # TODO: Make this work with new elastic search module - for obj in obj_list: - self.add(obj) - return # Group all objects by their type type_set = {} for obj in obj_list: @@ -299,11 +296,19 @@ class ElasticSearch(BaseSearch): type_set[obj_type].append(obj.indexed_build_document()) # Loop through each type and bulk add them - results = [] for type_name, type_objects in type_set.items(): - results.append((type_name, len(type_objects))) - self.es.bulk_index(self.es_index, type_name, type_objects) - return results + # Get list of actions + actions = [] + for obj in type_objects: + action = { + '_index': self.es_index, + '_type': type_name, + '_id': obj['id'], + } + action.update(obj) + actions.append(action) + + bulk(self.es, actions) def delete(self, obj): # Object must be a decendant of Indexed and be a django model From 598e6193da90908241937fe1a3fe9ab27983df36 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Sun, 6 Apr 2014 18:07:53 +0100 Subject: [PATCH 35/48] Install elasticsearch instead of elasticutils on travis Conflicts: .travis.yml --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 3e0a4bb98..8764f2859 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,7 +12,7 @@ services: # Package installation install: - python setup.py install - - pip install psycopg2 pyelasticsearch elasticutils==0.8.2 wand embedly + - pip install psycopg2 elasticsearch wand embedly - pip install coveralls # Pre-test configuration before_script: From 05cb87e3eb99c07764f176359d3ad488e020bb37 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Tue, 8 Apr 2014 10:47:13 +0100 Subject: [PATCH 36/48] Tests now succeed on elasticsearch 0.90.x --- .../wagtailsearch/backends/elasticsearch.py | 34 ++++++++++++------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/wagtail/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index 78de0e94f..d626d1d1c 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -2,7 +2,7 @@ from __future__ import absolute_import from django.db import models -from elasticsearch import Elasticsearch, NotFoundError +from elasticsearch import Elasticsearch, NotFoundError, RequestError from elasticsearch.helpers import bulk from wagtail.wagtailsearch.backends.base import BaseSearch @@ -99,38 +99,48 @@ class ElasticSearchResults(object): filters = self._get_filters() return { - 'query': { - 'filtered': { - 'query': query, - 'filter': { - 'and': filters, - } + 'filtered': { + 'query': query, + 'filter': { + 'and': filters, } } } def _get_results_pks(self, offset=0, limit=None): query = self._get_query() - query['query']['from'] = offset + query['from'] = offset if limit is not None: - query['query']['size'] = limit + query['size'] = limit hits = self.backend.es.search( index=self.backend.es_index, - body=query, + body=dict(query=query), _source=False, fields='pk', ) - return [hit['fields']['pk'][0] for hit in hits['hits']['hits']] + pks = [hit['fields']['pk'] for hit in hits['hits']['hits']] + + # ElasticSearch 1.x likes to pack pks into lists, unpack them if this has happened + return [pk[0] if isinstance(pk, list) else pk for pk in pks] def _get_count(self): query = self._get_query() + + # Elasticsearch 1.x count = self.backend.es.count( index=self.backend.es_index, - body=query, + body=dict(query=query), ) + # ElasticSearch 0.90.x fallback + if not count['_shards']['successful'] and "No query registered for [query]]" in count['_shards']['failures'][0]['reason']: + count = self.backend.es.count( + index=self.backend.es_index, + body=query, + ) + return count['count'] def __getitem__(self, key): From 8cb94f0b5c37df70624453ca0a7a1b8269f8973b Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Fri, 20 Jun 2014 13:46:43 +0100 Subject: [PATCH 37/48] Update frontenddevelopers.rst Added notes about new image manipulation techniques --- .../building_your_site/frontenddevelopers.rst | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index a8cf6a25e..bb12c5922 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -186,7 +186,24 @@ The available resizing methods are: More control over the ``img`` tag --------------------------------- -In some cases greater control over the ``img`` tag is required, for example to add a custom ``class``. Rather than generating the ``img`` element for you, Wagtail can assign the relevant data to another object using Django's ``as`` syntax: +Greater control over the ``img`` tag is often required, for example to add a custom ``class``. This can be most easily achieved in two ways: + +.. versionadded:: 0.4 + +Adding attributes to the {% image %} tag +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Simply add extra attributes with the syntax ``attribute="value"`` to the tag: + +.. code-block:: django + + {% image self.photo width-400 class="foo" id="bar" %} + + +Generating the image "as" +~~~~~~~~~~~~~~~~~~~~~~~~~ + +Wagtail can assign the image data to another object using Django's ``as`` syntax: .. code-block:: django @@ -197,12 +214,17 @@ In some cases greater control over the ``img`` tag is required, for example to a {{ tmp_photo.alt }} + +The ``attrs`` shortcut +----------------------- + You can also use the ``attrs`` property as a shorthand to output the ``src``, ``width``, ``height`` and ``alt`` attributes in one go: .. code-block:: django + .. _rich-text-filter: Rich text (filter) From c1f2f745afcd99cefccc9d9ac0f7615a2a73c69d Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Fri, 20 Jun 2014 13:49:25 +0100 Subject: [PATCH 38/48] Update frontenddevelopers.rst heading level fix --- docs/building_your_site/frontenddevelopers.rst | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index bb12c5922..7b674bf0d 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -190,8 +190,7 @@ Greater control over the ``img`` tag is often required, for example to add a cus .. versionadded:: 0.4 -Adding attributes to the {% image %} tag -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +**Adding attributes to the {% image %} tag** Simply add extra attributes with the syntax ``attribute="value"`` to the tag: @@ -200,8 +199,8 @@ Simply add extra attributes with the syntax ``attribute="value"`` to the tag: {% image self.photo width-400 class="foo" id="bar" %} -Generating the image "as" -~~~~~~~~~~~~~~~~~~~~~~~~~ +**Generating the image "as"** + Wagtail can assign the image data to another object using Django's ``as`` syntax: From 135d4f20a622196caf33c4eb2ac8294b5e8b7f66 Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Fri, 20 Jun 2014 14:04:11 +0100 Subject: [PATCH 39/48] Update frontenddevelopers.rst --- docs/building_your_site/frontenddevelopers.rst | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index 7b674bf0d..602c53c7f 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -186,34 +186,32 @@ The available resizing methods are: More control over the ``img`` tag --------------------------------- -Greater control over the ``img`` tag is often required, for example to add a custom ``class``. This can be most easily achieved in two ways: +Wagtail provides two shorcuts to gain grater greater control over the ``img`` element: .. versionadded:: 0.4 - **Adding attributes to the {% image %} tag** -Simply add extra attributes with the syntax ``attribute="value"`` to the tag: +Extra attributes can be specified with the syntax ``attribute="value"``: .. code-block:: django {% image self.photo width-400 class="foo" id="bar" %} +No validation is performed on attributes add in this way by the developer. It's possible to add `src`, `width`, `height` and `alt` of your own that might conflict with those generated by the tag itself. + **Generating the image "as"** - Wagtail can assign the image data to another object using Django's ``as`` syntax: .. code-block:: django - {% load image %} - ... {% image self.photo width-400 as tmp_photo %} {{ tmp_photo.alt }} - +.. versionadded:: 0.4 The ``attrs`` shortcut ----------------------- From ba0805e521986329a7ec4f55bde934aec30e678e Mon Sep 17 00:00:00 2001 From: Dave Cranwell Date: Fri, 20 Jun 2014 14:07:33 +0100 Subject: [PATCH 40/48] Update frontenddevelopers.rst typo fix --- docs/building_your_site/frontenddevelopers.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index 602c53c7f..8d0c9a3bb 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -186,7 +186,7 @@ The available resizing methods are: More control over the ``img`` tag --------------------------------- -Wagtail provides two shorcuts to gain grater greater control over the ``img`` element: +Wagtail provides two shorcuts to give greater control over the ``img`` element: .. versionadded:: 0.4 **Adding attributes to the {% image %} tag** From 55cd5aad835f604583e30cfd2375358d08e08606 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 20 Jun 2014 14:08:49 +0100 Subject: [PATCH 41/48] Revert frame-writing stuff from #315, as it fails when the preview loads before the placeholder - see https://github.com/torchbox/wagtail/pull/315#issuecomment-46669595 --- .../static/wagtailadmin/js/page-editor.js | 19 +++++-------------- .../templates/wagtailadmin/pages/preview.html | 1 - wagtail/wagtailadmin/urls.py | 1 - wagtail/wagtailadmin/views/pages.py | 6 ------ 4 files changed, 5 insertions(+), 22 deletions(-) diff --git a/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js b/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js index eacb8f86d..03adb7aca 100644 --- a/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js +++ b/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js @@ -329,9 +329,9 @@ $(function() { }); /* Set up behaviour of preview button */ - $('.action-preview').click(function(e) { + $('.action-preview').click(function(e) { e.preventDefault(); - + var previewWindow = window.open($(this).data('placeholder'), $(this).data('windowname')); $.ajax({ @@ -340,18 +340,9 @@ $(function() { data: $('#page-edit-form').serialize(), success: function(data, textStatus, request) { if (request.getResponseHeader('X-Wagtail-Preview') == 'ok') { - var pdoc = previewWindow.document; - var frame = pdoc.getElementById('preview-frame'); - - frame = frame.contentWindow || frame.contentDocument.document || frame.contentDocument; - frame.document.open(); - frame.document.write(data); - frame.document.close(); - - var hideTimeout = setTimeout(function(){ - pdoc.getElementById('loading-spinner-wrapper').className += 'remove'; - clearTimeout(hideTimeout); - }, 50) // just enough to give effect without adding discernible slowness + previewWindow.document.open(); + previewWindow.document.write(data); + previewWindow.document.close(); } else { previewWindow.close(); document.open(); diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/preview.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/preview.html index d5c4b7a51..46a4d2867 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/preview.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/preview.html @@ -11,6 +11,5 @@
- diff --git a/wagtail/wagtailadmin/urls.py b/wagtail/wagtailadmin/urls.py index 21f127c1d..ee68f4655 100644 --- a/wagtail/wagtailadmin/urls.py +++ b/wagtail/wagtailadmin/urls.py @@ -50,7 +50,6 @@ urlpatterns += [ url(r'^pages/(\d+)/edit/preview/$', pages.preview_on_edit, name='wagtailadmin_pages_preview_on_edit'), url(r'^pages/preview/$', pages.preview, name='wagtailadmin_pages_preview'), - url(r'^pages/preview_loading/$', pages.preview_loading, name='wagtailadmin_pages_preview_loading'), url(r'^pages/(\d+)/view_draft/$', pages.view_draft, name='wagtailadmin_pages_view_draft'), url(r'^pages/(\d+)/add_subpage/$', pages.add_subpage, name='wagtailadmin_pages_add_subpage'), diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index bfca64463..f215eb213 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -420,12 +420,6 @@ def preview(request): """ return render(request, 'wagtailadmin/pages/preview.html') -def preview_loading(request): - """ - This page is blank, but must be real HTML so its DOM can be written to once the preview of the page has rendered - """ - return HttpResponse("") - @permission_required('wagtailadmin.access_admin') def unpublish(request, page_id): page = get_object_or_404(Page, id=page_id) From 2afb13667fa99150bc6cbeaa52061f97dfd2aae6 Mon Sep 17 00:00:00 2001 From: jmiguelv Date: Thu, 19 Jun 2014 10:48:51 +0100 Subject: [PATCH 42/48] Added a new hook to add new element rules to the Whitelister. --- wagtail/wagtailcore/whitelist.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailcore/whitelist.py b/wagtail/wagtailcore/whitelist.py index a3d377bd6..0cfbbd2db 100644 --- a/wagtail/wagtailcore/whitelist.py +++ b/wagtail/wagtailcore/whitelist.py @@ -5,6 +5,8 @@ specific rules. from bs4 import BeautifulSoup, NavigableString, Tag from urlparse import urlparse +from wagtail.wagtailadmin import hooks + ALLOWED_URL_SCHEMES = ['', 'http', 'https', 'ftp', 'mailto', 'tel'] @@ -65,7 +67,8 @@ class Whitelister(object): 'h6': allow_without_attributes, 'hr': allow_without_attributes, 'i': allow_without_attributes, - 'img': attribute_rule({'src': check_url, 'width': True, 'height': True, 'alt': True}), + 'img': attribute_rule({'src': check_url, 'width': True, 'height': True, + 'alt': True}), 'li': allow_without_attributes, 'ol': allow_without_attributes, 'p': allow_without_attributes, @@ -77,7 +80,12 @@ class Whitelister(object): @classmethod def clean(cls, html): - """Clean up an HTML string to contain just the allowed elements / attributes""" + """Clean up an HTML string to contain just the allowed elements / + attributes""" + for fn in hooks.get_hooks('construct_whitelister_element_rules'): + cls.element_rules = dict( + cls.element_rules.items() + fn().items()) + doc = BeautifulSoup(html, 'lxml') cls.clean_node(doc, doc) return unicode(doc) From 9bc770d0a65386b81955d7660e14af00d9230ed3 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 20 Jun 2014 14:50:10 +0100 Subject: [PATCH 43/48] Add unit tests for DbWhitelister, including ability to override the whitelist via hooks --- wagtail/tests/wagtail_hooks.py | 9 ++++ .../wagtailcore/tests/test_dbwhitelister.py | 43 +++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 wagtail/wagtailcore/tests/test_dbwhitelister.py diff --git a/wagtail/tests/wagtail_hooks.py b/wagtail/tests/wagtail_hooks.py index 91c76c9f4..5dccca666 100644 --- a/wagtail/tests/wagtail_hooks.py +++ b/wagtail/tests/wagtail_hooks.py @@ -1,4 +1,5 @@ from wagtail.wagtailadmin import hooks +from wagtail.wagtailcore.whitelist import attribute_rule, check_url, allow_without_attributes def editor_css(): return """""" @@ -8,3 +9,11 @@ hooks.register('insert_editor_css', editor_css) def editor_js(): return """""" hooks.register('insert_editor_js', editor_js) + + +def whitelister_element_rules(): + return { + 'blockquote': allow_without_attributes, + 'a': attribute_rule({'href': check_url, 'target': True}), + } +hooks.register('construct_whitelister_element_rules', whitelister_element_rules) diff --git a/wagtail/wagtailcore/tests/test_dbwhitelister.py b/wagtail/wagtailcore/tests/test_dbwhitelister.py new file mode 100644 index 000000000..db5b931ee --- /dev/null +++ b/wagtail/wagtailcore/tests/test_dbwhitelister.py @@ -0,0 +1,43 @@ +from django.test import TestCase +from wagtail.wagtailcore.rich_text import DbWhitelister + +from bs4 import BeautifulSoup + +class TestDbWhitelister(TestCase): + def assertHtmlEqual(self, str1, str2): + """ + Assert that two HTML strings are equal at the DOM level + (necessary because we can't guarantee the order that attributes are output in) + """ + self.assertEqual(BeautifulSoup(str1), BeautifulSoup(str2)) + + def test_page_link_is_rewritten(self): + input_html = '

Look at the lovely homepage of my Wagtail site

' + output_html = DbWhitelister.clean(input_html) + expected = '

Look at the lovely homepage of my Wagtail site

' + self.assertHtmlEqual(expected, output_html) + + def test_document_link_is_rewritten(self): + input_html = '

Look at our horribly oversized brochure

' + output_html = DbWhitelister.clean(input_html) + expected = '

Look at our horribly oversized brochure

' + self.assertHtmlEqual(expected, output_html) + + def test_image_embed_is_rewritten(self): + input_html = '

OMG look at this picture of a kitten:

A cute kitten
A kitten, yesterday.

' + output_html = DbWhitelister.clean(input_html) + expected = '

OMG look at this picture of a kitten:

' + self.assertHtmlEqual(expected, output_html) + + def test_media_embed_is_rewritten(self): + input_html = '

OMG look at this video of a kitten:

' + output_html = DbWhitelister.clean(input_html) + expected = '

OMG look at this video of a kitten:

' + self.assertHtmlEqual(expected, output_html) + + def test_whitelist_hooks(self): + # wagtail.tests.wagtail_hooks overrides the whitelist to permit
and + input_html = '
I would put a tax on all people who stand in water.

- Gumby' + output_html = DbWhitelister.clean(input_html) + expected = '

I would put a tax on all people who stand in water.

- Gumby' + self.assertHtmlEqual(expected, output_html) From 505a1291a80a3a7fb088004b98b5192afa561d01 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 20 Jun 2014 15:07:58 +0100 Subject: [PATCH 44/48] Move construct_whitelister_element_rules hook logic into DbWhitelister, as the whitelist module should be Wagtail-agnostic --- wagtail/wagtailcore/rich_text.py | 14 ++++++++++++++ wagtail/wagtailcore/tests/test_dbwhitelister.py | 11 +++++++++-- wagtail/wagtailcore/whitelist.py | 6 ------ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/wagtail/wagtailcore/rich_text.py b/wagtail/wagtailcore/rich_text.py index aa8f25336..8004cb30d 100644 --- a/wagtail/wagtailcore/rich_text.py +++ b/wagtail/wagtailcore/rich_text.py @@ -13,6 +13,8 @@ from wagtail.wagtaildocs.models import Document from wagtail.wagtailimages.models import get_image_model from wagtail.wagtailimages.formats import get_image_format +from wagtail.wagtailadmin import hooks + # Define a set of 'embed handlers' and 'link handlers'. These handle the translation # of 'special' HTML elements in rich text - ones which we do not want to include @@ -158,6 +160,18 @@ LINK_HANDLERS = { # Prepare a whitelisting engine with custom behaviour: # rewrite any elements with a data-embedtype or data-linktype attribute class DbWhitelister(Whitelister): + has_loaded_custom_whitelist_rules = False + + @classmethod + def clean(cls, html): + if not cls.has_loaded_custom_whitelist_rules: + for fn in hooks.get_hooks('construct_whitelister_element_rules'): + cls.element_rules = dict( + cls.element_rules.items() + fn().items()) + cls.has_loaded_custom_whitelist_rules = True + + return super(DbWhitelister, cls).clean(html) + @classmethod def clean_tag_node(cls, doc, tag): if 'data-embedtype' in tag.attrs: diff --git a/wagtail/wagtailcore/tests/test_dbwhitelister.py b/wagtail/wagtailcore/tests/test_dbwhitelister.py index db5b931ee..795b9637c 100644 --- a/wagtail/wagtailcore/tests/test_dbwhitelister.py +++ b/wagtail/wagtailcore/tests/test_dbwhitelister.py @@ -1,5 +1,6 @@ from django.test import TestCase from wagtail.wagtailcore.rich_text import DbWhitelister +from wagtail.wagtailcore.whitelist import Whitelister from bs4 import BeautifulSoup @@ -37,7 +38,13 @@ class TestDbWhitelister(TestCase): def test_whitelist_hooks(self): # wagtail.tests.wagtail_hooks overrides the whitelist to permit

and - input_html = '
I would put a tax on all people who stand in water.

- Gumby' + input_html = '

I would put a tax on all people who stand in water.

- Gumby

' output_html = DbWhitelister.clean(input_html) - expected = '
I would put a tax on all people who stand in water.

- Gumby' + expected = '

I would put a tax on all people who stand in water.

- Gumby

' + self.assertHtmlEqual(expected, output_html) + + # check that the base Whitelister class is unaffected by these custom whitelist rules + input_html = '
I would put a tax on all people who stand in water.

- Gumby

' + output_html = Whitelister.clean(input_html) + expected = 'I would put a tax on all people who stand in water.

- Gumby

' self.assertHtmlEqual(expected, output_html) diff --git a/wagtail/wagtailcore/whitelist.py b/wagtail/wagtailcore/whitelist.py index 0cfbbd2db..9a5f67cf1 100644 --- a/wagtail/wagtailcore/whitelist.py +++ b/wagtail/wagtailcore/whitelist.py @@ -5,8 +5,6 @@ specific rules. from bs4 import BeautifulSoup, NavigableString, Tag from urlparse import urlparse -from wagtail.wagtailadmin import hooks - ALLOWED_URL_SCHEMES = ['', 'http', 'https', 'ftp', 'mailto', 'tel'] @@ -82,10 +80,6 @@ class Whitelister(object): def clean(cls, html): """Clean up an HTML string to contain just the allowed elements / attributes""" - for fn in hooks.get_hooks('construct_whitelister_element_rules'): - cls.element_rules = dict( - cls.element_rules.items() + fn().items()) - doc = BeautifulSoup(html, 'lxml') cls.clean_node(doc, doc) return unicode(doc) From 1aab35ba24341bb497316ccd880f813a095c6792 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 20 Jun 2014 16:08:50 +0100 Subject: [PATCH 45/48] add documentation for the construct_whitelister_element_rules hook --- docs/editing_api.rst | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/docs/editing_api.rst b/docs/editing_api.rst index 88fc9458f..9f7602b11 100644 --- a/docs/editing_api.rst +++ b/docs/editing_api.rst @@ -546,6 +546,28 @@ Where ``'hook'`` is one of the following hook strings and ``function`` is a func + 'demo/css/vendor/font-awesome/css/font-awesome.min.css">') hooks.register('insert_editor_css', editor_css) +.. _construct_whitelister_element_rules: + +``construct_whitelister_element_rules`` + .. versionadded:: 0.4 + Customise the rules that define which HTML elements are allowed in rich text areas. By default only a limited set of HTML elements and attributes are whitelisted - all others are stripped out. The callables passed into this hook must return a dict, which maps element names to handler functions that will perform some kind of manipulation of the element. These handler functions receive the element as a `BeautifulSoup `_ Tag object. + + The ``wagtail.wagtailcore.whitelist`` module provides a few helper functions to assist in defining these handlers: ``allow_without_attributes``, a handler which preserves the element but strips out all of its attributes, and ``attribute_rule`` which accepts a dict specifying how to handle each attribute, and returns a handler function. This dict will map attribute names to either True (indicating that the attribute should be kept), False (indicating that it should be dropped), or a callable (which takes the initial attribute value and returns either a final value for the attribute, or None to drop the attribute). + + For example, the following hook function will add the ``
`` element to the whitelist, and allow the ``target`` attribute on ```` elements: + + .. code-block:: python + + from wagtail.wagtailadmin import hooks + from wagtail.wagtailcore.whitelist import attribute_rule, check_url, allow_without_attributes + + def whitelister_element_rules(): + return { + 'blockquote': allow_without_attributes, + 'a': attribute_rule({'href': check_url, 'target': True}), + } + hooks.register('construct_whitelister_element_rules', whitelister_element_rules) + Image Formats in the Rich Text Editor ------------------------------------- From f4c51d8205f4c3a9cc5146021761e7d9f1d20671 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Fri, 20 Jun 2014 16:10:03 +0100 Subject: [PATCH 46/48] add changelog entry for construct_whitelister_element_rules hook --- CHANGELOG.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 19eec21e7..e5cda8b44 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -14,6 +14,7 @@ Changelog * Aesthetic improvements to preview experience * 'image' tag now accepts extra keyword arguments to be output as attributes on the img tag * Added an 'attrs' property to image rendition objects to output src, width, height and alt attributes all in one go + * Added 'construct_whitelister_element_rules' hook for customising the HTML whitelist used when saving rich text fields * Fix: Animated GIFs are now coalesced before resizing * Fix: Wand backend clones images before modifying them * Fix: Admin breadcrumb now positioned correctly on mobile From 2801c913dc3b64d41cfd5ed70ebab270e434f9df Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 23 Jun 2014 14:22:45 +0100 Subject: [PATCH 47/48] Docs update for #342 merge --- docs/wagtail_search.rst | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/docs/wagtail_search.rst b/docs/wagtail_search.rst index 3d8ea26b2..1fa2c6525 100644 --- a/docs/wagtail_search.rst +++ b/docs/wagtail_search.rst @@ -220,17 +220,14 @@ The default DB search backend uses Django's ``__icontains`` filter. Elasticsearch Backend ````````````````````` -Prerequisites are the Elasticsearch service itself and, via pip, the `elasticutils`_ and `pyelasticsearch`_ packages: +Prerequisites are the Elasticsearch service itself and, via pip, the `elasticsearch-py`_ package: .. code-block:: guess - pip install elasticutils==0.8.2 pyelasticsearch + pip install elasticsearch .. note:: - ElasticUtils 0.9+ is not supported. - -.. note:: - The dependency on elasticutils and pyelasticsearch is scheduled to be replaced by a dependency on `elasticsearch-py`_. + If you are using Elasticsearch < 1.0, install elasticsearch-py version 0.4.5: ```pip install elasticsearch==0.4.5``` The backend is configured in settings: @@ -246,7 +243,7 @@ The backend is configured in settings: } } -Other than ``BACKEND`` the keys are optional and default to the values shown. ``FORCE_NEW`` is used by elasticutils. In addition, any other keys are passed directly to the Elasticsearch constructor as case-sensitive keyword arguments (e.g. ``'max_retries': 1``). +Other than ``BACKEND`` the keys are optional and default to the values shown. ``FORCE_NEW`` is used by elasticsearch-py. In addition, any other keys are passed directly to the Elasticsearch constructor as case-sensitive keyword arguments (e.g. ``'max_retries': 1``). If you prefer not to run an Elasticsearch server in development or production, there are many hosted services available, including `Searchly`_, who offer a free account suitable for testing and development. To use Searchly: @@ -256,8 +253,6 @@ If you prefer not to run an Elasticsearch server in development or production, t - Configure ``URLS`` and ``INDEX`` in the Elasticsearch entry in ``WAGTAILSEARCH_BACKENDS`` - Run ``./manage.py update_index`` -.. _elasticutils: http://elasticutils.readthedocs.org -.. _pyelasticsearch: http://pyelasticsearch.readthedocs.org .. _elasticsearch-py: http://elasticsearch-py.readthedocs.org .. _Searchly: http://www.searchly.com/ .. _dashboard.searchly.com/users/sign\_up: https://dashboard.searchly.com/users/sign_up From 765657329e529f828e0bbf0185c162570301a539 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 23 Jun 2014 14:28:36 +0100 Subject: [PATCH 48/48] Changelog entry for #342 --- CHANGELOG.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index e5cda8b44..ca58949b4 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -3,6 +3,7 @@ Changelog 0.4 (xx.xx.20xx) ~~~~~~~~~~~~~~~~ + * ElasticUtils/pyelasticsearch swapped for elasticsearch-py * Added 'original' as a resizing rule supported by the 'image' tag * Hallo.js updated to version 1.0.4 * Snippets are now ordered alphabetically