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: diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 039935b43..ca58949b4 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -3,15 +3,19 @@ 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 * 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 + * '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 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 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 diff --git a/docs/building_your_site/frontenddevelopers.rst b/docs/building_your_site/frontenddevelopers.rst index 0ef918582..651b15818 100644 --- a/docs/building_your_site/frontenddevelopers.rst +++ b/docs/building_your_site/frontenddevelopers.rst @@ -189,17 +189,41 @@ 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: +Wagtail provides two shorcuts to give greater control over the ``img`` element: + +.. versionadded:: 0.4 +**Adding attributes to the {% image %} tag** + +Extra attributes can be specified with the syntax ``attribute="value"``: .. code-block:: django - - {% load wagtailimages_tags %} - ... + + {% 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 + {% image self.photo width-400 as tmp_photo %} {{ tmp_photo.alt }} +.. versionadded:: 0.4 +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: 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 ------------------------------------- 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 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/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/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/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/wagtailadmin/edit_handlers.py b/wagtail/wagtailadmin/edit_handlers.py index 2b4006610..a15440016 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/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/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/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 09e6b6f5c..191c51a50 100644 --- a/wagtail/wagtailadmin/tests/test_pages_views.py +++ b/wagtail/wagtailadmin/tests/test_pages_views.py @@ -1,10 +1,11 @@ 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 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): @@ -85,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 @@ -104,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 @@ -127,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 @@ -144,7 +215,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 @@ -165,6 +236,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) @@ -245,8 +319,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) @@ -263,8 +336,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) @@ -287,8 +359,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) @@ -302,6 +373,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!", @@ -354,8 +448,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) @@ -514,9 +607,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) @@ -554,8 +646,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) @@ -606,8 +697,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) @@ -681,41 +771,87 @@ 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() 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 + 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) + + 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/tests/test_userbar.py b/wagtail/wagtailadmin/tests/test_userbar.py new file mode 100644 index 000000000..acdb81690 --- /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_frontend_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_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_moderation_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) 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 c3eb24e9e..b6677d7ec 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') @@ -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: @@ -59,6 +57,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, @@ -111,15 +115,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) @@ -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 }) @@ -420,12 +422,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) diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 5c3f9d1c5..6b273e4b6 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 @@ -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): @@ -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) 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 new file mode 100644 index 000000000..795b9637c --- /dev/null +++ b/wagtail/wagtailcore/tests/test_dbwhitelister.py @@ -0,0 +1,50 @@ +from django.test import TestCase +from wagtail.wagtailcore.rich_text import DbWhitelister +from wagtail.wagtailcore.whitelist import Whitelister + +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) + + # 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/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('_') diff --git a/wagtail/wagtailcore/whitelist.py b/wagtail/wagtailcore/whitelist.py index a3d377bd6..9a5f67cf1 100644 --- a/wagtail/wagtailcore/whitelist.py +++ b/wagtail/wagtailcore/whitelist.py @@ -65,7 +65,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 +78,8 @@ 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""" doc = BeautifulSoup(html, 'lxml') cls.clean_node(doc, doc) return unicode(doc) diff --git a/wagtail/wagtaildocs/tests.py b/wagtail/wagtaildocs/tests.py index 3353ce39e..a8420406b 100644 --- a/wagtail/wagtaildocs/tests.py +++ b/wagtail/wagtaildocs/tests.py @@ -43,29 +43,61 @@ 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") + 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'] for ordering in orderings: - response = self.get({'ordering': ordering}) + response = self.client.get(reverse('wagtaildocs_index'), {'ordering': ordering}) self.assertEqual(response.status_code, 200) @@ -73,33 +105,63 @@ 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') - # 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() - # Create a document to edit - self.document = models.Document.objects.create(title="Test document") + # Build a fake file + fake_file = ContentFile("A boring example document") + fake_file.name = 'test.txt' - def get(self, params={}): - return self.client.get(reverse('wagtaildocs_edit_document', args=(self.document.id,)), params) + # Create a document to edit + self.document = models.Document.objects.create(title="Test document", file=fake_file) 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') - # 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): @@ -109,40 +171,80 @@ 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') - # 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): 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") + 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): @@ -152,31 +254,40 @@ 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') - # TODO: Test posting - 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') - # 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): diff --git a/wagtail/wagtailimages/models.py b/wagtail/wagtailimages/models.py index 8137971a7..3ff2f9782 100644 --- a/wagtail/wagtailimages/models.py +++ b/wagtail/wagtailimages/models.py @@ -10,7 +10,7 @@ from django.db import models from django.db.models.signals import pre_delete from django.dispatch.dispatcher import receiver from django.utils.safestring import mark_safe -from django.utils.html import escape +from django.utils.html import escape, format_html_join from django.conf import settings from django.utils.translation import ugettext_lazy as _ @@ -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) @@ -236,11 +236,19 @@ class AbstractRendition(models.Model): def url(self): return self.file.url - def img_tag(self): + @property + def attrs(self): return mark_safe( - '%s' % (escape(self.url), self.width, self.height, escape(self.image.title)) + 'src="%s" width="%d" height="%d" alt="%s"' % (escape(self.url), self.width, self.height, escape(self.image.title)) ) + def img_tag(self, extra_attributes=None): + if extra_attributes: + extra_attributes_string = format_html_join(' ', '{0}="{1}"', extra_attributes.items()) + return mark_safe('' % (self.attrs, extra_attributes_string)) + else: + return mark_safe('' % self.attrs) + class Meta: abstract = True diff --git a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py index e59d9cd14..5c7273417 100644 --- a/wagtail/wagtailimages/templatetags/wagtailimages_tags.py +++ b/wagtail/wagtailimages/templatetags/wagtailimages_tags.py @@ -7,32 +7,36 @@ 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: - # 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: + if len(bits) == 2 and bits[0] == 'as': # 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 %%}") - - return ImageNode(image_var, filter_spec, out_var) - + return ImageNode(image_var, filter_spec, output_var_name=bits[1]) else: - raise template.TemplateSyntaxError("'image' tag should be of the form {%% image self.photo max-320x200 %%} or {%% image self.photo max-320x200 as img %%}") + # 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 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) 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 +70,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) diff --git a/wagtail/wagtailimages/tests.py b/wagtail/wagtailimages/tests.py index b31e33745..2fbac118c 100644 --- a/wagtail/wagtailimages/tests.py +++ b/wagtail/wagtailimages/tests.py @@ -197,6 +197,32 @@ 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) + + 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 ===== @@ -253,8 +279,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 +318,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 +352,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/wagtailsearch/backends/elasticsearch.py b/wagtail/wagtailsearch/backends/elasticsearch.py index d2b58d564..d626d1d1c 100644 --- a/wagtail/wagtailsearch/backends/elasticsearch.py +++ b/wagtail/wagtailsearch/backends/elasticsearch.py @@ -1,6 +1,9 @@ +from __future__ import absolute_import + from django.db import models -from elasticutils import get_es, S +from elasticsearch import Elasticsearch, NotFoundError, RequestError +from elasticsearch.helpers import bulk from wagtail.wagtailsearch.backends.base import BaseSearch from wagtail.wagtailsearch.indexed import Indexed @@ -9,16 +12,141 @@ 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 { + 'filtered': { + 'query': query, + 'filter': { + 'and': filters, + } + } + } + + def _get_results_pks(self, offset=0, limit=None): + query = self._get_query() + query['from'] = offset + if limit is not None: + query['size'] = limit + + hits = self.backend.es.search( + index=self.backend.es_index, + body=dict(query=query), + _source=False, + fields='pk', + ) + + 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=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): 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 +173,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 +192,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 +251,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 +267,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 @@ -183,24 +306,33 @@ 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 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 +347,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) diff --git a/wagtail/wagtailsearch/tests/test_editorspicks.py b/wagtail/wagtailsearch/tests/test_editorspicks.py index 7ee974fd0..99debab40 100644 --- a/wagtail/wagtailsearch/tests/test_editorspicks.py +++ b/wagtail/wagtailsearch/tests/test_editorspicks.py @@ -1,4 +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 @@ -49,39 +51,104 @@ 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 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): - 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_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): 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') - # 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): @@ -90,17 +157,123 @@ 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") - - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/', params) + 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.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') - # 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': 1, + '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_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 = { + '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': 1, + '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': 1, + '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): @@ -109,14 +282,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") - - def get(self, params={}): - return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/delete/', params) + 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.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') - # 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()) 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): 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)