Merge branch 'master' into feature/minor-preview-improvement

.travis.yml

@@ -12,7 +12,7 @@ services:
 # Package installation
 install:
   - python setup.py install
-  - pip install psycopg2 pyelasticsearch elasticutils==0.8.2 wand
+  - pip install psycopg2 pyelasticsearch elasticutils==0.8.2 wand embedly
   - pip install coveralls
 # Pre-test configuration
 before_script:

CHANGELOG.txt

@@ -8,6 +8,7 @@ Changelog
  * 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
  * Fix: Animated GIFs are now coalesced before resizing
  * Fix: Wand backend clones images before modifying them
  * Fix: Admin breadcrumb now positioned correctly on mobile

CONTRIBUTORS.rst

@@ -28,6 +28,7 @@ Contributors
 * Ben Margolis
 * Tom Talbot
 * Jeffrey Hearn
+* Robert Clark
 
 Translators
 ===========

docs/editing_api.rst

@@ -371,8 +371,8 @@ Edit Handler API
 ~~~~~~~~~~~~~~~~
 
 
-Hooks
------
+Admin Hooks
+-----------
 
 On loading, Wagtail will search for any app with the file ``wagtail_hooks.py`` and execute the contents. This provides a way to register your own functions to execute at certain points in Wagtail's execution, such as when a ``Page`` object is saved or when the main menu is constructed.
 
@@ -547,6 +547,38 @@ Where ``'hook'`` is one of the following hook strings and ``function`` is a func
     hooks.register('insert_editor_css', editor_css)
 
 
+Image Formats in the Rich Text Editor
+-------------------------------------
+
+On loading, Wagtail will search for any app with the file ``image_formats.py`` and execute the contents. This provides a way to customize the formatting options shown to the editor when inserting images in the ``RichTextField`` editor.
+
+As an example, add a "thumbnail" format:
+
+.. code-block:: python
+  # image_formats.py
+  from wagtail.wagtailimages.formats import Format, register_image_format
+
+  register_image_format(Format('thumbnail', 'Thumbnail', 'richtext-image thumbnail', 'max-120x120'))
+
+
+To begin, import the the ``Format`` class, ``register_image_format`` function, and optionally ``unregister_image_format`` function. To register a new ``Format``, call the ``register_image_format`` with the ``Format`` object as the argument. The ``Format`` takes the following init arguments:
+
+``name``
+  The unique key used to identify the format. To unregister this format, call ``unregister_image_format`` with this string as the only argument.
+
+``label``
+  The label used in the chooser form when inserting the image into the ``RichTextField``.
+
+``classnames``
+  The string to assign to the ``class`` attribute of the generated ``<img>`` tag.
+
+``filter_spec``
+  The string specification to create the image rendition. For more, see the :ref:`image_tag`.
+
+
+To unregister, call ``unregister_image_format`` with the string of the ``name`` of the ``Format`` as the only argument.
+
+
 Content Index Pages (CRUD)
 --------------------------
 

docs/gettingstarted.rst

@@ -65,7 +65,7 @@ Wagtail instance available as the basis for your new site:
 -  Install `Vagrant <http://www.vagrantup.com/>`_ 1.1+
 -  Clone the demonstration site, create the Vagrant box and initialise Wagtail::
 
-	git clone git@github.com:torchbox/wagtaildemo.git
+	git clone https://github.com/torchbox/wagtaildemo.git
 	cd wagtaildemo
 	vagrant up
 	vagrant ssh

docs/wagtail_search.rst

@@ -224,10 +224,13 @@ Prerequisites are the Elasticsearch service itself and, via pip, the `elasticuti
 
 .. code-block:: guess
 
-  pip install elasticutils pyelasticsearch
+  pip install elasticutils==0.8.2 pyelasticsearch
 
 .. note::
-    The dependency on pyelasticsearch is scheduled to be replaced by a dependency on `elasticsearch-py`_.
+    ElasticUtils 0.9+ is not supported.
+
+.. note::
+    The dependency on elasticutils and pyelasticsearch is scheduled to be replaced by a dependency on `elasticsearch-py`_.
 
 The backend is configured in settings:
 

runtests.py

@@ -101,7 +101,9 @@ if not settings.configured:
         ),
         COMPRESS_ENABLED=False,  # disable compression so that we can run tests on the content of the compress tag
         WAGTAILSEARCH_BACKENDS=WAGTAILSEARCH_BACKENDS,
-        WAGTAIL_SITE_NAME='Test Site'
+        WAGTAIL_SITE_NAME='Test Site',
+        LOGIN_REDIRECT_URL='wagtailadmin_home',
+        LOGIN_URL='wagtailadmin_login',
     )
 
 

wagtail/tests/models.py

@@ -294,3 +294,16 @@ class ZuluSnippet(models.Model):
 
     def __unicode__(self):
         return self.text
+
+
+class StandardIndex(Page):
+    pass
+
+class StandardChild(Page):
+    pass
+
+class BusinessIndex(Page):
+    subpage_types = ['tests.BusinessChild']
+
+class BusinessChild(Page):
+    subpage_types = []

wagtail/tests/utils.py

@@ -1,4 +1,7 @@
+from django.test import TestCase
 from django.contrib.auth.models import User
+from django.utils.six.moves.urllib.parse import urlparse, ParseResult
+from django.http import QueryDict
 
 # We need to make sure that we're using the same unittest library that Django uses internally
 # Otherwise, we get issues with the "SkipTest" and "ExpectedFailure" exceptions being recognised as errors
@@ -12,11 +15,30 @@ except ImportError:
     import unittest
 
 
-def login(client):
-    # Create a user
-    user = User.objects.create_superuser(username='test', email='test@email.com', password='password')
+class WagtailTestUtils(object):
+    def login(self):
+        # Create a user
+        user = User.objects.create_superuser(username='test', email='test@email.com', password='password')
 
-    # Login
-    client.login(username='test', password='password')
+        # Login
+        self.client.login(username='test', password='password')
 
-    return user
+        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))

wagtail/wagtailadmin/static/wagtailadmin/js/modal-workflow.js

@@ -16,9 +16,12 @@ function ModalWorkflow(opts) {
     /* remove any previous modals before continuing (closing doesn't remove them from the dom) */
     $('body > .modal').remove();
 
+    // set default contents of container
     var container = $('<div class="modal fade" tabindex="-1" role="dialog" aria-hidden="true">\n    <div class="modal-dialog">\n        <div class="modal-content">\n            <button type="button" class="close icon text-replace icon-cross" data-dismiss="modal" aria-hidden="true">&times;</button>\n            <div class="modal-body"></div>\n        </div><!-- /.modal-content -->\n    </div><!-- /.modal-dialog -->\n</div>');
+    
+    // add container to body and hide it, so content can be added to it before display
     $('body').append(container);
-    container.modal();
+    container.modal('hide');
 
     self.body = container.find('.modal-body');
 
@@ -49,15 +52,19 @@ function ModalWorkflow(opts) {
 
     self.loadResponseText = function(responseText) {
         var response = eval('(' + responseText + ')');
+
         self.loadBody(response);
     };
 
-    self.loadBody = function(body) {
-        if (body.html) {
-            self.body.html(body.html);
+    self.loadBody = function(response) {
+        if (response.html) {
+            // if the response is html
+            self.body.html(response.html);
+            container.modal('show');
         }
-        if (body.onload) {
-            body.onload(self);
+        if (response.onload) {
+            // if the response is a function
+            response.onload(self);
         }
     };
 

wagtail/wagtailadmin/static/wagtailadmin/js/vendor/bootstrap-transition.js

@@ -0,0 +1,65 @@
+/* ========================================================================
+ * Bootstrap: transition.js v3.1.1
+ * http://getbootstrap.com/javascript/#transitions
+ * ========================================================================
+ * Copyright 2011-2014 Twitter, Inc.
+ * Licensed under MIT (https://github.com/twbs/bootstrap/blob/master/LICENSE)
+ * ======================================================================== */
+
+
++function () { 'use strict';
+
+  (function (o_o) {
+    typeof define  == 'function' && define.amd ? define(['jquery'], o_o) :
+    typeof exports == 'object' ? o_o(require('jquery')) : o_o(jQuery)
+  })(function ($) {
+
+    // CSS TRANSITION SUPPORT (Shoutout: http://www.modernizr.com/)
+    // ============================================================
+
+    function transitionEnd() {
+      var el = document.createElement('bootstrap')
+
+      var transEndEventNames = {
+        WebkitTransition : 'webkitTransitionEnd',
+        MozTransition    : 'transitionend',
+        OTransition      : 'oTransitionEnd otransitionend',
+        transition       : 'transitionend'
+      }
+
+      for (var name in transEndEventNames) {
+        if (el.style[name] !== undefined) {
+          return { end: transEndEventNames[name] }
+        }
+      }
+
+      return false // explicit for ie8 (  ._.)
+    }
+
+    // http://blog.alexmaccaw.com/css-transitions
+    $.fn.emulateTransitionEnd = function (duration) {
+      var called = false
+      var $el = this
+      $(this).one('bsTransitionEnd', function () { called = true })
+      var callback = function () { if (!called) $($el).trigger($.support.transition.end) }
+      setTimeout(callback, duration)
+      return this
+    }
+
+    $(function () {
+      $.support.transition = transitionEnd()
+
+      if (!$.support.transition) return
+
+      $.event.special.bsTransitionEnd = {
+        bindType: $.support.transition.end,
+        delegateType: $.support.transition.end,
+        handle: function (e) {
+          if ($(e.target).is(this)) return e.handleObj.handler.apply(this, arguments)
+        }
+      }
+    })
+
+  })
+
+}();

wagtail/wagtailadmin/static/wagtailadmin/scss/components/forms.scss

@@ -648,6 +648,13 @@ ul.tagit li.tagit-choice-editable{
     }
 }
 
+/* search-bars */
+.search-bar{
+    .required label:after{
+        display:none;
+    }
+}
+
 /* Transitions */
 fieldset, input, textarea, select{
     @include transition(background-color 0.2s ease);

wagtail/wagtailadmin/static/wagtailadmin/scss/components/modals.scss

@@ -1,5 +1,15 @@
 $zindex-modal-background: 500;
 
+.fade {
+    opacity: 0;
+    @include transition(opacity .15s linear);
+    
+    &.in {
+        opacity: 1;
+    }
+}
+
+
 // Kill the scroll on the body
 .modal-open {
     overflow: hidden;
@@ -70,7 +80,7 @@ $zindex-modal-background: 500;
     background-color: black;
     // Fade for backdrop
     &.fade { opacity:0; }
-    &.in { opacity:0.7; }
+    &.in { opacity:0.5; }
 }
 
 .modal .close{

wagtail/wagtailadmin/templates/wagtailadmin/account/password_reset/complete.html

@@ -13,6 +13,6 @@
 {% block furniture %}
     <div class="content-wrapper">
         <h1>{% trans "Password change successful" %}</h1>
-        <p><a href="{% url 'django.contrib.auth.views.login' %}" class="button button-primary">{% trans "Login" %}</a></p>
+        <p><a href="{% url 'wagtailadmin_login' %}" class="button button-primary">{% trans "Login" %}</a></p>
     </div>
 {% endblock %}

wagtail/wagtailadmin/templates/wagtailadmin/admin_base.html

@@ -18,6 +18,7 @@
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/jquery-ui-1.10.3.js"></script>
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/jquery.timepicker.min.js"></script>
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/jquery.autosize.js"></script>
+        <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/bootstrap-transition.js"></script>
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/bootstrap-modal.js"></script>
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/bootstrap-tab.js"></script>
         <script src="{{ STATIC_URL }}wagtailadmin/js/vendor/jquery.dlmenu.js"></script>

wagtail/wagtailadmin/templates/wagtailadmin/login.html

@@ -20,7 +20,7 @@
             </div>
         {% endif %}
 
-        <form action="{% url 'django.contrib.auth.views.login' %}" method="post" autocomplete="off">
+        <form action="{% url 'wagtailadmin_login' %}" method="post" autocomplete="off">
             {% csrf_token %}
             <input type="hidden" name="next" value="{{ next }}" />
             <h1>{% trans "Sign in to Wagtail" %}</h1>

wagtail/wagtailadmin/templates/wagtailadmin/pages/add_subpage.html

@@ -12,9 +12,9 @@
     <div class="nice-padding">
         <p>{% trans "Choose which type of page you'd like to create." %}</p>
 
-        {% if all_page_types %}
+        {% if page_types %}
             <ul class="listing">
-                {% for content_type in all_page_types %}
+                {% for content_type in page_types %}
                     <li>
                         <div class="row row-flush">
                             <div class="col6">

wagtail/wagtailadmin/templates/wagtailadmin/pages/create.html

@@ -40,7 +40,7 @@
                 <li class="actions preview">
                     {% trans 'Preview' as preview_label %}
                     {% if display_modes|length > 1 %}
-                        <div class="dropdown dropup button match-width">
+                        <div class="dropdown dropup dropdown-button match-width">
                             {% include "wagtailadmin/pages/_preview_button_on_create.html" with label=preview_label icon=1 %}
                             <div class="dropdown-toggle icon icon-arrow-up"></div>
                             <ul role="menu">

wagtail/wagtailadmin/templates/wagtailadmin/shared/field_as_li.html

@@ -1,5 +1,5 @@
 {% load wagtailadmin_tags %}
-<li class="{{ field.css_classes }} {{ field|fieldtype }} {{ li_classes }} {% if field.errors %}error{% endif %}">
+<li class="{% if field.field.required %}required{% endif %} {{ field.css_classes }} {{ field|fieldtype }} {{ li_classes }} {% if field.errors %}error{% endif %}">
     <div class="field">
         {{ field.label_tag }}
         <div class="field-content">

wagtail/wagtailadmin/tests/test_account_management.py

@@ -1,17 +1,17 @@
 from django.test import TestCase
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
 from django.contrib.auth.tokens import PasswordResetTokenGenerator
 from django.core import mail
 
 
-class TestAuthentication(TestCase):
+class TestAuthentication(TestCase, WagtailTestUtils):
     """
     This tests that users can login and logout of the admin interface
     """
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def test_login_view(self):
         """
@@ -44,12 +44,12 @@ class TestAuthentication(TestCase):
 
         # Check that the user was redirected to the dashboard
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailadmin_home'))
 
         # Check that the user was logged in
         self.assertTrue('_auth_user_id' in self.client.session)
         self.assertEqual(self.client.session['_auth_user_id'], User.objects.get(username='test').id)
 
-    @unittest.expectedFailure # See: https://github.com/torchbox/wagtail/issues/25
     def test_already_logged_in_redirect(self):
         """
         This tests that a user who is already logged in is automatically
@@ -61,27 +61,63 @@ class TestAuthentication(TestCase):
 
         # Check that the user was redirected to the dashboard
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailadmin_home'))
 
     def test_logout(self):
         """
         This tests that the user can logout
         """
-        # Get logout page page
+        # Get logout page
         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'))
 
         # Check that the user was logged out
         self.assertFalse('_auth_user_id' in self.client.session)
 
+    def test_not_logged_in_redirect(self):
+        """
+        This tests that a not logged in user is redirected to the
+        login page
+        """
+        # Logout
+        self.client.logout()
 
-class TestAccountSection(TestCase):
+        # Get dashboard
+        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'))
+
+    def test_not_logged_in_redirect_default_settings(self):
+        """
+        This does the same as the above test but checks that it
+        redirects to the correct place when the user has not set
+        the LOGIN_URL setting correctly
+        """
+        # Logout
+        self.client.logout()
+
+        # Get dashboard with default LOGIN_URL setting
+        with self.settings(LOGIN_URL='django.contrib.auth.views.login'):
+            response = self.client.get(reverse('wagtailadmin_home'))
+
+        # Check that the user was redirected to the login page and that next was set correctly
+        # 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'))
+
+
+class TestAccountSection(TestCase, WagtailTestUtils):
     """
     This tests that the accounts section is working
     """
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def test_account_view(self):
         """
@@ -117,8 +153,9 @@ class TestAccountSection(TestCase):
         }
         response = self.client.post(reverse('wagtailadmin_account_change_password'), post_data)
 
-        # Check that the user was redirected
+        # Check that the user was redirected to the account page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailadmin_account'))
 
         # Check that the password was changed
         self.assertTrue(User.objects.get(username='test').check_password('newpassword'))
@@ -146,7 +183,7 @@ class TestAccountSection(TestCase):
         self.assertTrue(User.objects.get(username='test').check_password('password'))
 
 
-class TestPasswordReset(TestCase):
+class TestPasswordReset(TestCase, WagtailTestUtils):
     """
     This tests that the password reset is working
     """
@@ -176,8 +213,9 @@ class TestPasswordReset(TestCase):
         }
         response = self.client.post(reverse('password_reset'), post_data)
 
-        # Check that the user was redirected
+        # Check that the user was redirected to the done page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('password_reset_done'))
 
         # Check that a password reset email was sent to the user
         self.assertEqual(len(mail.outbox), 1)
@@ -267,8 +305,9 @@ class TestPasswordReset(TestCase):
         }
         response = self.client.post(reverse('password_reset_confirm', kwargs=self.url_kwargs), post_data)
 
-        # Check that the user was redirected
+        # Check that the user was redirected to the complete page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('password_reset_complete'))
 
         # Check that the password was changed
         self.assertTrue(User.objects.get(username='test').check_password('newpassword'))

wagtail/wagtailadmin/tests/test_page_chooser.py

@@ -3,9 +3,10 @@ from django.core.urlresolvers import reverse
 
 from wagtail.wagtailcore.models import Page
 from wagtail.tests.models import SimplePage
-from wagtail.tests.utils import login
+from wagtail.tests.utils import WagtailTestUtils
 
-class TestChooserBrowse(TestCase):
+
+class TestChooserBrowse(TestCase, WagtailTestUtils):
     def setUp(self):
         self.root_page = Page.objects.get(id=2)
 
@@ -15,13 +16,15 @@ class TestChooserBrowse(TestCase):
         self.child_page.slug = "foobarbaz"
         self.root_page.add_child(instance=self.child_page)
 
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailadmin_choose_page'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailadmin/chooser/browse.html')
 
     def test_search(self):
         response = self.get({'q': "foobarbaz"})
@@ -39,7 +42,7 @@ class TestChooserBrowse(TestCase):
         self.assertEqual(response.status_code, 404)
 
 
-class TestChooserBrowseChild(TestCase):
+class TestChooserBrowseChild(TestCase, WagtailTestUtils):
     def setUp(self):
         self.root_page = Page.objects.get(id=2)
 
@@ -49,7 +52,7 @@ class TestChooserBrowseChild(TestCase):
         self.child_page.slug = "foobarbaz"
         self.root_page.add_child(instance=self.child_page)
 
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailadmin_choose_page_child',
@@ -59,8 +62,10 @@ class TestChooserBrowseChild(TestCase):
         return self.client.get(reverse('wagtailadmin_choose_page_child',
                                        args=(9999999,)), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailadmin/chooser/browse.html')
 
     def test_search(self):
         response = self.get({'q': "foobarbaz"})
@@ -77,9 +82,9 @@ class TestChooserBrowseChild(TestCase):
         self.assertEqual(self.get_invalid().status_code, 404)
 
 
-class TestChooserExternalLink(TestCase):
+class TestChooserExternalLink(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailadmin_choose_page_external_link'), params)
@@ -87,8 +92,10 @@ class TestChooserExternalLink(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailadmin_choose_page_external_link'), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailadmin/chooser/external_link.html')
 
     def test_get_with_param(self):
         self.assertEqual(self.get({'prompt_for_link_text': 'foo'}).status_code, 200)
@@ -99,9 +106,9 @@ class TestChooserExternalLink(TestCase):
         self.assertContains(request, "'title': 'http://www.example.com/'")
 
 
-class TestChooserEmailLink(TestCase):
+class TestChooserEmailLink(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailadmin_choose_page_email_link'), params)
@@ -109,8 +116,10 @@ class TestChooserEmailLink(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailadmin_choose_page_email_link'), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailadmin/chooser/email_link.html')
 
     def test_get_with_param(self):
         self.assertEqual(self.get({'prompt_for_link_text': 'foo'}).status_code, 200)

wagtail/wagtailadmin/tests/test_pages_views.py

@@ -1,13 +1,13 @@
 from django.test import TestCase
-from wagtail.tests.models import SimplePage, EventPage
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.models import SimplePage, EventPage, StandardIndex, StandardChild, BusinessIndex, BusinessChild
+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
 
 
-class TestPageExplorer(TestCase):
+class TestPageExplorer(TestCase, WagtailTestUtils):
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
@@ -19,7 +19,7 @@ class TestPageExplorer(TestCase):
         self.root_page.add_child(instance=self.child_page)
 
         # Login
-        login(self.client)
+        self.login()
 
     def test_explore(self):
         response = self.client.get(reverse('wagtailadmin_explore', args=(self.root_page.id, )))
@@ -28,13 +28,13 @@ class TestPageExplorer(TestCase):
         self.assertTrue(response.context['pages'].paginator.object_list.filter(id=self.child_page.id).exists())
 
 
-class TestPageCreation(TestCase):
+class TestPageCreation(TestCase, WagtailTestUtils):
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
 
         # Login
-        self.user = login(self.client)
+        self.user = self.login()
 
     def test_add_subpage(self):
         response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.root_page.id, )))
@@ -86,6 +86,7 @@ class TestPageCreation(TestCase):
 
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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,6 +105,7 @@ class TestPageCreation(TestCase):
 
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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
@@ -126,6 +128,7 @@ class TestPageCreation(TestCase):
 
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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
@@ -185,8 +188,13 @@ class TestPageCreation(TestCase):
         self.assertTemplateUsed(response, 'tests/simple_page.html')
         self.assertContains(response, "New page!")
 
+        # Check that the treebeard attributes were set correctly on the page object
+        self.assertEqual(response.context['self'].depth, self.root_page.depth + 1)
+        self.assertTrue(response.context['self'].path.startswith(self.root_page.path))
+        self.assertEqual(response.context['self'].get_parent(), self.root_page)
 
-class TestPageEdit(TestCase):
+
+class TestPageEdit(TestCase, WagtailTestUtils):
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
@@ -206,7 +214,7 @@ class TestPageEdit(TestCase):
         self.root_page.add_child(instance=self.event_page)
 
         # Login
-        self.user = login(self.client)
+        self.user = self.login()
 
     def test_page_edit(self):
         # Tests that the edit page loads
@@ -238,6 +246,7 @@ class TestPageEdit(TestCase):
     
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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)
@@ -255,6 +264,7 @@ class TestPageEdit(TestCase):
     
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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)
@@ -278,6 +288,7 @@ class TestPageEdit(TestCase):
     
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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)
@@ -306,7 +317,7 @@ class TestPageEdit(TestCase):
         self.assertContains(response, "I've been edited!")
 
 
-class TestPageDelete(TestCase):
+class TestPageDelete(TestCase, WagtailTestUtils):
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
@@ -318,7 +329,7 @@ class TestPageDelete(TestCase):
         self.root_page.add_child(instance=self.child_page)
 
         # Login
-        self.user = login(self.client)
+        self.user = self.login()
 
     def test_page_delete(self):
         response = self.client.get(reverse('wagtailadmin_pages_delete', args=(self.child_page.id, )))
@@ -344,15 +355,16 @@ class TestPageDelete(TestCase):
 
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, 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)
 
 
-class TestPageSearch(TestCase):
+class TestPageSearch(TestCase, WagtailTestUtils):
     def setUp(self):
         # Login
-        login(self.client)
+        self.login()
 
     def get(self, params=None, **extra):
         return self.client.get(reverse('wagtailadmin_pages_search'), params or {}, **extra)
@@ -390,7 +402,7 @@ class TestPageSearch(TestCase):
         self.assertTrue(any([r.slug == 'root' for r in results]))
 
 
-class TestPageMove(TestCase):
+class TestPageMove(TestCase, WagtailTestUtils):
     def setUp(self):
         # Find root page
         self.root_page = Page.objects.get(id=2)
@@ -413,7 +425,7 @@ class TestPageMove(TestCase):
         self.section_a.add_child(instance=self.test_page)
 
         # Login
-        self.user = login(self.client)
+        self.user = self.login()
 
     def test_page_move(self):
         response = self.client.get(reverse('wagtailadmin_pages_move', args=(self.test_page.id, )))
@@ -442,18 +454,18 @@ class TestPageMove(TestCase):
         self.assertEqual(response.status_code, 200)
 
 
-class TestPageUnpublish(TestCase):
+class TestPageUnpublish(TestCase, WagtailTestUtils):
     def setUp(self):
-        self.user = login(self.client)
+        self.user = self.login()
 
         # Create a page to unpublish
-        root_page = Page.objects.get(id=2)
+        self.root_page = Page.objects.get(id=2)
         self.page = SimplePage(
             title="Hello world!",
             slug='hello-world',
             live=True,
         )
-        root_page.add_child(instance=self.page)
+        self.root_page.add_child(instance=self.page)
 
     def test_unpublish_view(self):
         """
@@ -502,14 +514,15 @@ class TestPageUnpublish(TestCase):
             'foo': "Must post something or the view won't see this as a POST request",
         })
 
-        # Check that the user was redirected
+        # 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, )))
 
         # Check that the page was unpublished
         self.assertFalse(SimplePage.objects.get(id=self.page.id).live)
 
 
-class TestApproveRejectModeration(TestCase):
+class TestApproveRejectModeration(TestCase, WagtailTestUtils):
     def setUp(self):
         self.submitter = User.objects.create_superuser(
             username='submitter',
@@ -517,7 +530,7 @@ class TestApproveRejectModeration(TestCase):
             password='password',
         )
 
-        self.user = login(self.client)
+        self.user = self.login()
 
         # Create a page and submit it for moderation
         root_page = Page.objects.get(id=2)
@@ -540,8 +553,9 @@ class TestApproveRejectModeration(TestCase):
             'foo': "Must post something or the view won't see this as a POST request",
         })
 
-        # Check that the user was redirected
+        # Check that the user was redirected to the dashboard
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailadmin_home'))
 
         # Page must be live
         self.assertTrue(Page.objects.get(id=self.page.id).live)
@@ -591,8 +605,9 @@ class TestApproveRejectModeration(TestCase):
             'foo': "Must post something or the view won't see this as a POST request",
         })
 
-        # Check that the user was redirected
+        # Check that the user was redirected to the dashboard
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailadmin_home'))
 
         # Page must not be live
         self.assertFalse(Page.objects.get(id=self.page.id).live)
@@ -645,11 +660,11 @@ class TestApproveRejectModeration(TestCase):
         self.assertContains(response, "Hello world!")
 
 
-class TestContentTypeUse(TestCase):
+class TestContentTypeUse(TestCase, WagtailTestUtils):
     fixtures = ['test.json']
 
     def setUp(self):
-        self.user = login(self.client)
+        self.user = self.login()
 
     def test_content_type_use(self):
         # Get use of event page
@@ -659,3 +674,48 @@ class TestContentTypeUse(TestCase):
         self.assertEqual(response.status_code, 200)
         self.assertTemplateUsed(response, 'wagtailadmin/pages/content_type_use.html')
         self.assertContains(response, "Christmas")
+
+
+class TestSubpageBusinessRules(TestCase, WagtailTestUtils):
+    def setUp(self):
+        # Find root page
+        self.root_page = Page.objects.get(id=2)
+
+        # Add standard page
+        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
+        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
+        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)
+
+        # Login
+        self.login()
+
+    def test_standard_subpage(self):
+        response = self.client.get(reverse('wagtailadmin_pages_add_subpage', args=(self.standard_index.id, )))
+        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, )))
+        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, )))
+        self.assertEqual(response.status_code, 200)
+        self.assertNotContains(response, 'Standard Child')
+        self.assertEqual(0, len(response.context['page_types']))

wagtail/wagtailadmin/tests/tests.py

@@ -1,26 +1,26 @@
 from django.test import TestCase
 from wagtail.tests.models import SimplePage, EventPage
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.wagtailcore.models import Page
 from wagtail.wagtailadmin.tasks import send_email_task
 from django.core.urlresolvers import reverse
 from django.core import mail
 
 
-class TestHome(TestCase):
+class TestHome(TestCase, WagtailTestUtils):
     def setUp(self):
         # Login
-        login(self.client)
+        self.login()
 
-    def test_status_code(self):
+    def test_simple(self):
         response = self.client.get(reverse('wagtailadmin_home'))
         self.assertEqual(response.status_code, 200)
 
 
-class TestEditorHooks(TestCase):
+class TestEditorHooks(TestCase, WagtailTestUtils):
     def setUp(self):
         self.homepage = Page.objects.get(id=2)
-        login(self.client)
+        self.login()
 
     def test_editor_css_and_js_hooks_on_add(self):
         response = self.client.get(reverse('wagtailadmin_pages_create', args=('tests', 'simplepage', self.homepage.id)))

wagtail/wagtailadmin/urls.py

@@ -5,15 +5,8 @@ from wagtail.wagtailadmin.forms import LoginForm, PasswordResetForm
 from wagtail.wagtailadmin.views import account, chooser, home, pages, tags, userbar
 from wagtail.wagtailadmin import hooks
 
-urlpatterns = [
-    url(
-        r'^login/$', 'django.contrib.auth.views.login', {
-            'template_name': 'wagtailadmin/login.html',
-            'authentication_form': LoginForm,
-            'extra_context': {'show_password_reset': getattr(settings, 'WAGTAIL_PASSWORD_MANAGEMENT_ENABLED', True)},
-        }, name='wagtailadmin_login'
-    ),
 
+urlpatterns = [
     # Password reset
     url(
         r'^password_reset/$', 'django.contrib.auth.views.password_reset', {
@@ -82,6 +75,7 @@ urlpatterns += [
 
     url(r'^tag-autocomplete/$', tags.autocomplete, name='wagtailadmin_tag_autocomplete'),
 
+    url(r'^login/$', account.login, name='wagtailadmin_login'),
     url(r'^account/$', account.account, name='wagtailadmin_account'),
     url(r'^account/change_password/$', account.change_password, name='wagtailadmin_account_change_password'),
     url(r'^logout/$', account.logout, name='wagtailadmin_logout'),
@@ -91,6 +85,13 @@ urlpatterns += [
 ]
 
 
+# This is here to make sure that 'django.contrib.auth.views.login' is reversed correctly
+# It must be placed after 'wagtailadmin_login' to prevent this from being used
+urlpatterns += [
+    url(r'^login/$', 'django.contrib.auth.views.login'),
+]
+
+
 # Import additional urlpatterns from any apps that define a register_admin_urls hook
 for fn in hooks.get_hooks('register_admin_urls'):
     urls = fn()

wagtail/wagtailadmin/views/account.py

@@ -3,8 +3,13 @@ from django.shortcuts import render, redirect
 from django.contrib import messages
 from django.contrib.auth.forms import SetPasswordForm
 from django.contrib.auth.decorators import permission_required
-from django.contrib.auth.views import logout as auth_logout
+from django.contrib.auth.views import logout as auth_logout, login as auth_login
 from django.utils.translation import ugettext as _ 
+from django.views.decorators.debug import sensitive_post_parameters
+from django.views.decorators.cache import never_cache
+
+from wagtail.wagtailadmin import forms
+
 
 @permission_required('wagtailadmin.access_admin')
 def account(request):
@@ -37,6 +42,21 @@ def change_password(request):
     })
 
 
+@sensitive_post_parameters()
+@never_cache
+def login(request):
+    if request.user.is_authenticated():
+        return redirect('wagtailadmin_home')
+    else:
+        return auth_login(request,
+            template_name='wagtailadmin/login.html',
+            authentication_form=forms.LoginForm,
+            extra_context={
+                'show_password_reset': getattr(settings, 'WAGTAIL_PASSWORD_MANAGEMENT_ENABLED', True),
+            },
+        )
+
+
 def logout(request):
     response = auth_logout(request, next_page = 'wagtailadmin_login')
 

wagtail/wagtailadmin/views/pages.py

@@ -57,13 +57,11 @@ def add_subpage(request, parent_page_id):
     if not parent_page.permissions_for_user(request.user).can_add_subpage():
         raise PermissionDenied
 
-    page_types = sorted([ContentType.objects.get_for_model(model_class) for model_class in parent_page.clean_subpage_types()], key=lambda pagetype: pagetype.name.lower())
-    all_page_types = sorted(get_page_types(), key=lambda pagetype: pagetype.name.lower())
+    page_types = sorted(parent_page.clean_subpage_types(), key=lambda pagetype: pagetype.name.lower())
 
     return render(request, 'wagtailadmin/pages/add_subpage.html', {
         'parent_page': parent_page,
         'page_types': page_types,
-        'all_page_types': all_page_types,
     })
 
 
@@ -364,6 +362,10 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p
         parent_page = get_object_or_404(Page, id=parent_page_id).specific
         page.set_url_path(parent_page)
 
+        # Set treebeard attributes
+        page.depth = parent_page.depth + 1
+        page.path = Page._get_children_path_interval(parent_page.path)[1]
+
         # This view will generally be invoked as an AJAX request; as such, in the case of
         # an error Django will return a plaintext response. This isn't what we want, since
         # we will be writing the response back to an HTML page regardless of success or

wagtail/wagtailcore/models.py

@@ -1,7 +1,6 @@
-import sys
-import os
 from StringIO import StringIO
 from urlparse import urlparse
+import warnings
 
 from modelcluster.models import ClusterableModel
 
@@ -101,30 +100,29 @@ def get_page_types():
     return _PAGE_CONTENT_TYPES
 
 
-LEAF_PAGE_MODEL_CLASSES = []
-_LEAF_PAGE_CONTENT_TYPE_IDS = []
-
-
 def get_leaf_page_content_type_ids():
-    global _LEAF_PAGE_CONTENT_TYPE_IDS
-    if len(_LEAF_PAGE_CONTENT_TYPE_IDS) != len(LEAF_PAGE_MODEL_CLASSES):
-        _LEAF_PAGE_CONTENT_TYPE_IDS = [
-            ContentType.objects.get_for_model(cls).id for cls in LEAF_PAGE_MODEL_CLASSES
-        ]
-    return _LEAF_PAGE_CONTENT_TYPE_IDS
-
-
-NAVIGABLE_PAGE_MODEL_CLASSES = []
-_NAVIGABLE_PAGE_CONTENT_TYPE_IDS = []
-
+    warnings.warn("""
+        get_leaf_page_content_type_ids is deprecated, as it treats pages without an explicit subpage_types
+        setting as 'leaf' pages. Code that calls get_leaf_page_content_type_ids must be rewritten to avoid
+        this incorrect assumption.
+    """, DeprecationWarning)
+    return [
+        content_type.id
+        for content_type in get_page_types()
+        if not getattr(content_type.model_class(), 'subpage_types', None)
+    ]
 
 def get_navigable_page_content_type_ids():
-    global _NAVIGABLE_PAGE_CONTENT_TYPE_IDS
-    if len(_NAVIGABLE_PAGE_CONTENT_TYPE_IDS) != len(NAVIGABLE_PAGE_MODEL_CLASSES):
-        _NAVIGABLE_PAGE_CONTENT_TYPE_IDS = [
-            ContentType.objects.get_for_model(cls).id for cls in NAVIGABLE_PAGE_MODEL_CLASSES
-        ]
-    return _NAVIGABLE_PAGE_CONTENT_TYPE_IDS
+    warnings.warn("""
+        get_navigable_page_content_type_ids is deprecated, as it treats pages without an explicit subpage_types
+        setting as 'leaf' pages. Code that calls get_navigable_page_content_type_ids must be rewritten to avoid
+        this incorrect assumption.
+    """, DeprecationWarning)
+    return [
+        content_type.id
+        for content_type in get_page_types()
+        if getattr(content_type.model_class(), 'subpage_types', None)
+    ]
 
 
 class PageManager(models.Manager):
@@ -209,10 +207,6 @@ class PageBase(models.base.ModelBase):
         if not cls.is_abstract:
             # register this type in the list of page content types
             PAGE_MODEL_CLASSES.append(cls)
-        if cls.subpage_types:
-            NAVIGABLE_PAGE_MODEL_CLASSES.append(cls)
-        else:
-            LEAF_PAGE_MODEL_CLASSES.append(cls)
 
 
 class Page(MP_Node, ClusterableModel, Indexed):
@@ -259,9 +253,6 @@ class Page(MP_Node, ClusterableModel, Indexed):
     def __unicode__(self):
         return self.title
 
-    # by default pages do not allow any kind of subpages
-    subpage_types = []
-
     is_abstract = True  # don't offer Page in the list of page types a superuser can create
 
     def set_url_path(self, parent):
@@ -416,10 +407,10 @@ class Page(MP_Node, ClusterableModel, Indexed):
     def is_navigable(self):
         """
         Return true if it's meaningful to browse subpages of this page -
-        i.e. it currently has subpages, or its page type indicates that sub-pages are supported,
+        i.e. it currently has subpages,
         or it's at the top level (this rule necessary for empty out-of-the-box sites to have working navigation)
         """
-        return (not self.is_leaf()) or (self.content_type_id not in get_leaf_page_content_type_ids()) or self.depth == 2
+        return (not self.is_leaf()) or self.depth == 2
 
     def get_other_siblings(self):
         # get sibling pages excluding self
@@ -484,25 +475,30 @@ class Page(MP_Node, ClusterableModel, Indexed):
             where required
         """
         if cls._clean_subpage_types is None:
-            res = []
-            for page_type in cls.subpage_types:
-                if isinstance(page_type, basestring):
-                    try:
-                        app_label, model_name = page_type.split(".")
-                    except ValueError:
-                        # If we can't split, assume a model in current app
-                        app_label = cls._meta.app_label
-                        model_name = page_type
+            subpage_types = getattr(cls, 'subpage_types', None)
+            if subpage_types is None:
+                # if subpage_types is not specified on the Page class, allow all page types as subpages
+                res = get_page_types()
+            else:
+                res = []
+                for page_type in cls.subpage_types:
+                    if isinstance(page_type, basestring):
+                        try:
+                            app_label, model_name = page_type.split(".")
+                        except ValueError:
+                            # If we can't split, assume a model in current app
+                            app_label = cls._meta.app_label
+                            model_name = page_type
+
+                        model = get_model(app_label, model_name)
+                        if model:
+                            res.append(ContentType.objects.get_for_model(model))
+                        else:
+                            raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
 
-                    model = get_model(app_label, model_name)
-                    if model:
-                        res.append(model)
                     else:
-                        raise NameError(_("name '{0}' (used in subpage_types list) is not defined.").format(page_type))
-
-                else:
-                    # assume it's already a model class
-                    res.append(page_type)
+                        # assume it's already a model class
+                        res.append(ContentType.objects.get_for_model(page_type))
 
             cls._clean_subpage_types = res
 
@@ -652,13 +648,8 @@ class Page(MP_Node, ClusterableModel, Indexed):
 
 def get_navigation_menu_items():
     # Get all pages that appear in the navigation menu: ones which have children,
-    # or are a non-leaf type (indicating that they *could* have children),
     # or are at the top-level (this rule required so that an empty site out-of-the-box has a working menu)
-    navigable_content_type_ids = get_navigable_page_content_type_ids()
-    if navigable_content_type_ids:
-        pages = Page.objects.filter(Q(content_type__in=navigable_content_type_ids)|Q(depth=2)|Q(numchild__gt=0)).order_by('path')
-    else:
-        pages = Page.objects.filter(Q(depth=2)|Q(numchild__gt=0)).order_by('path')
+    pages = Page.objects.filter(Q(depth=2)|Q(numchild__gt=0)).order_by('path')
 
     # Turn this into a tree structure:
     #     tree_node = (page, children)

wagtail/wagtailcore/tests/test_whitelist.py

@@ -0,0 +1,136 @@
+from bs4 import BeautifulSoup, NavigableString
+
+from django.test import TestCase
+from wagtail.wagtailcore.whitelist import (
+    check_url,
+    attribute_rule,
+    allow_without_attributes,
+    Whitelister
+)
+
+class TestCheckUrl(TestCase):
+    def test_allowed_url_schemes(self):
+        for url_scheme in ['', 'http', 'https', 'ftp', 'mailto', 'tel']:
+            url = url_scheme + "://www.example.com"
+            self.assertTrue(bool(check_url(url)))
+
+    def test_disallowed_url_scheme(self):
+        self.assertFalse(bool(check_url("invalid://url")))
+
+
+class TestAttributeRule(TestCase):
+    def setUp(self):
+        self.soup = BeautifulSoup('<b foo="bar">baz</b>')
+
+    def test_no_rule_for_attr(self):
+        """
+        Test that attribute_rule() drops attributes for
+        which no rule has been defined.
+        """
+        tag = self.soup.b
+        fn = attribute_rule({'snowman': 'barbecue'})
+        fn(tag)
+        self.assertEqual(str(tag), '<b>baz</b>')
+
+    def test_rule_true_for_attr(self):
+        """
+        Test that attribute_rule() does not change atrributes
+        when the corresponding rule returns True
+        """
+        tag = self.soup.b
+        fn = attribute_rule({'foo': True})
+        fn(tag)
+        self.assertEqual(str(tag), '<b foo="bar">baz</b>')
+
+    def test_rule_false_for_attr(self):
+        """
+        Test that attribute_rule() drops atrributes
+        when the corresponding rule returns False
+        """
+        tag = self.soup.b
+        fn = attribute_rule({'foo': False})
+        fn(tag)
+        self.assertEqual(str(tag), '<b>baz</b>')
+
+    def test_callable_called_on_attr(self):
+        """
+        Test that when the rule returns a callable,
+        attribute_rule() replaces the attribute with
+        the result of calling the callable on the attribute.
+        """
+        tag = self.soup.b
+        fn = attribute_rule({'foo': len})
+        fn(tag)
+        self.assertEqual(str(tag), '<b foo="3">baz</b>')
+
+    def test_callable_returns_None(self):
+        """
+        Test that when the rule returns a callable,
+        attribute_rule() replaces the attribute with
+        the result of calling the callable on the attribute.
+        """
+        tag = self.soup.b
+        fn = attribute_rule({'foo': lambda x: None})
+        fn(tag)
+        self.assertEqual(str(tag), '<b>baz</b>')
+
+    def test_allow_without_attributes(self):
+        """
+        Test that attribute_rule() with will drop all
+        attributes.
+        """
+        soup = BeautifulSoup('<b foo="bar" baz="quux" snowman="barbecue"></b>')
+        tag = soup.b
+        allow_without_attributes(tag)
+        self.assertEqual(str(tag), '<b></b>')
+
+
+class TestWhitelister(TestCase):
+    def test_clean_unknown_node(self):
+        """
+        Unknown node should remove a node from the parent document
+        """
+        soup = BeautifulSoup('<foo><bar>baz</bar>quux</foo>')
+        tag = soup.foo
+        Whitelister.clean_unknown_node('', soup.bar)
+        self.assertEqual(str(tag), '<foo>quux</foo>')
+
+    def test_clean_tag_node_cleans_nested_recognised_node(self):
+        """
+        <b> tags are allowed without attributes. This remains true
+        when tags are nested.
+        """
+        soup = BeautifulSoup('<b><b class="delete me">foo</b></b>')
+        tag = soup.b
+        Whitelister.clean_tag_node(tag, tag)
+        self.assertEqual(str(tag), '<b><b>foo</b></b>')
+
+    def test_clean_tag_node_disallows_nested_unrecognised_node(self):
+        """
+        <foo> tags should be removed, even when nested.
+        """
+        soup = BeautifulSoup('<b><foo>bar</foo></b>')
+        tag = soup.b
+        Whitelister.clean_tag_node(tag, tag)
+        self.assertEqual(str(tag), '<b>bar</b>')
+
+    def test_clean_string_node_does_nothing(self):
+        soup = BeautifulSoup('<b>bar</b>')
+        string = soup.b.string
+        Whitelister.clean_string_node(string, string)
+        self.assertEqual(str(string), 'bar')
+
+    def test_clean_node_does_not_change_navigable_strings(self):
+        soup = BeautifulSoup('<b>bar</b>')
+        string = soup.b.string
+        Whitelister.clean_node(string, string)
+        self.assertEqual(str(string), 'bar')
+
+    def test_clean(self):
+        """
+        Whitelister.clean should remove disallowed tags and attributes from
+        a string
+        """
+        string = '<b foo="bar">snowman <barbecue>Yorkshire</barbecue></b>'
+        cleaned_string = Whitelister.clean(string)
+        self.assertEqual(cleaned_string, '<b>snowman Yorkshire</b>')

wagtail/wagtailcore/whitelist.py

@@ -89,7 +89,10 @@ class Whitelister(object):
             cls.clean_string_node(doc, node)
         elif isinstance(node, Tag):
             cls.clean_tag_node(doc, node)
-        else:
+        # This branch is here in case node is a BeautifulSoup object that does
+        # not inherit from NavigableString or Tag. I can't find any examples
+        # of such a thing at the moment, so this branch is untested.
+        else:  # pragma: no cover
             cls.clean_unknown_node(doc, node)
 
     @classmethod

wagtail/wagtaildocs/tests.py

@@ -1,6 +1,6 @@
 from django.test import TestCase
 from wagtail.wagtaildocs import models
-from wagtail.tests.utils import login
+from wagtail.tests.utils import WagtailTestUtils
 from django.contrib.auth.models import User, Group, Permission
 from django.core.urlresolvers import reverse
 from django.core.files.base import ContentFile
@@ -39,15 +39,17 @@ class TestDocumentPermissions(TestCase):
 ## ===== ADMIN VIEWS =====
 
 
-class TestDocumentIndexView(TestCase):
+class TestDocumentIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_index'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtaildocs/documents/index.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -67,20 +69,24 @@ class TestDocumentIndexView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestDocumentAddView(TestCase):
+class TestDocumentAddView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_add_document'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtaildocs/documents/add.html')
+
+    # TODO: Test posting
 
 
-class TestDocumentEditView(TestCase):
+class TestDocumentEditView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create a document to edit
         self.document = models.Document.objects.create(title="Test document")
@@ -88,13 +94,17 @@ class TestDocumentEditView(TestCase):
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_edit_document', args=(self.document.id,)), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtaildocs/documents/edit.html')
+
+    # TODO: Test posting
 
 
-class TestDocumentDeleteView(TestCase):
+class TestDocumentDeleteView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create a document to delete
         self.document = models.Document.objects.create(title="Test document")
@@ -102,19 +112,26 @@ class TestDocumentDeleteView(TestCase):
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_delete_document', args=(self.document.id,)), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtaildocs/documents/confirm_delete.html')
+
+    # TODO: Test posting
 
 
-class TestDocumentChooserView(TestCase):
+class TestDocumentChooserView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_chooser'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        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"})
@@ -128,9 +145,9 @@ class TestDocumentChooserView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestDocumentChooserChosenView(TestCase):
+class TestDocumentChooserChosenView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create a document to choose
         self.document = models.Document.objects.create(title="Test document")
@@ -138,19 +155,28 @@ class TestDocumentChooserChosenView(TestCase):
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_document_chosen', args=(self.document.id,)), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtaildocs/chooser/document_chosen.js')
+
+    # TODO: Test posting
 
 
-class TestDocumentChooserUploadView(TestCase):
+class TestDocumentChooserUploadView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtaildocs_chooser_upload'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        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
 
 
 class TestDocumentFilenameProperties(TestCase):

wagtail/wagtailembeds/tests.py

@@ -8,10 +8,8 @@ except ImportError:
     no_embedly = True
 
 from django.test import TestCase
-from django.test.client import Client
 
-from wagtail.tests.utils import login
-from wagtail.tests.utils import unittest
+from wagtail.tests.utils import WagtailTestUtils, unittest
 
 from wagtail.wagtailembeds import get_embed
 from wagtail.wagtailembeds.embeds import (
@@ -19,8 +17,8 @@ from wagtail.wagtailembeds.embeds import (
     EmbedlyException,
     AccessDeniedEmbedlyException,
 )
-from wagtail.wagtailembeds.embeds import embedly as wagtail_embedly
-from wagtail.wagtailembeds.embeds import oembed as wagtail_oembed
+from wagtail.wagtailembeds.embeds import embedly as wagtail_embedly, oembed as wagtail_oembed
+
 
 
 class TestEmbeds(TestCase):
@@ -82,10 +80,10 @@ class TestEmbeds(TestCase):
         self.assertEqual(embed.width, None)
 
 
-class TestChooser(TestCase):
+class TestChooser(TestCase, WagtailTestUtils):
     def setUp(self):
         # login
-        login(self.client)
+        self.login()
 
     def test_chooser(self):
         r = self.client.get('/admin/embeds/chooser/')

wagtail/wagtailimages/templates/wagtailimages/chooser/chooser.js

@@ -1,4 +1,6 @@
 function(modal) {
+    var searchUrl = $('form.image-search', modal.body).attr('action');
+
     function ajaxifyLinks (context) {
         $('.listing a', context).click(function() {
             modal.loadUrl(this.href);
@@ -12,7 +14,6 @@ function(modal) {
         });
     }
 
-    var searchUrl = $('form.image-search', modal.body).attr('action');
     function search() {
         $.ajax({
             url: searchUrl,
@@ -24,8 +25,8 @@ function(modal) {
         });
         return false;
     }
-    function setPage(page) {
 
+    function setPage(page) {
         if($('#id_q').val().length){
             dataObj = {q: $('#id_q').val(), p: page};
         }else{

wagtail/wagtailimages/tests.py

@@ -4,7 +4,7 @@ from django.contrib.auth.models import User, Group, Permission
 from django.core.urlresolvers import reverse
 from django.core.files.uploadedfile import SimpleUploadedFile
 
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.wagtailimages.models import get_image_model
 from wagtail.wagtailimages.templatetags import image_tags
 
@@ -201,15 +201,17 @@ class TestImageTag(TestCase):
 ## ===== ADMIN VIEWS =====
 
 
-class TestImageIndexView(TestCase):
+class TestImageIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailimages_index'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/index.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -229,9 +231,9 @@ class TestImageIndexView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestImageAddView(TestCase):
+class TestImageAddView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailimages_add_image'), params)
@@ -239,8 +241,10 @@ class TestImageAddView(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailimages_add_image'), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/add.html')
 
     def test_add(self):
         response = self.post({
@@ -250,6 +254,7 @@ class TestImageAddView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailimages_index'))
 
         # Check that the image was created
         images = Image.objects.filter(title="Test image")
@@ -261,9 +266,9 @@ class TestImageAddView(TestCase):
         self.assertEqual(image.height, 480)
 
 
-class TestImageEditView(TestCase):
+class TestImageEditView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create an image to edit
         self.image = Image.objects.create(
@@ -277,8 +282,10 @@ class TestImageEditView(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailimages_edit_image', args=(self.image.id,)), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/edit.html')
 
     def test_edit(self):
         response = self.post({
@@ -287,15 +294,16 @@ class TestImageEditView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailimages_index'))
 
         # Check that the image was edited
         image = Image.objects.get(id=self.image.id)
         self.assertEqual(image.title, "Edited")
 
 
-class TestImageDeleteView(TestCase):
+class TestImageDeleteView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create an image to edit
         self.image = Image.objects.create(
@@ -309,8 +317,10 @@ class TestImageDeleteView(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailimages_delete_image', args=(self.image.id,)), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/images/confirm_delete.html')
 
     def test_delete(self):
         response = self.post({
@@ -319,21 +329,25 @@ class TestImageDeleteView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailimages_index'))
 
         # Check that the image was deleted
         images = Image.objects.filter(title="Test image")
         self.assertEqual(images.count(), 0)
 
 
-class TestImageChooserView(TestCase):
+class TestImageChooserView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailimages_chooser'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.js')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -347,9 +361,9 @@ class TestImageChooserView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestImageChooserChosenView(TestCase):
+class TestImageChooserChosenView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create an image to edit
         self.image = Image.objects.create(
@@ -360,16 +374,25 @@ class TestImageChooserChosenView(TestCase):
     def get(self, params={}):
         return self.client.get(reverse('wagtailimages_image_chosen', args=(self.image.id,)), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/image_chosen.js')
+
+    # TODO: Test posting
 
 
-class TestImageChooserUploadView(TestCase):
+class TestImageChooserUploadView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailimages_chooser_upload'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.html')
+        self.assertTemplateUsed(response, 'wagtailimages/chooser/chooser.js')
+
+    # TODO: Test uploading through chooser

wagtail/wagtailredirects/tests.py

@@ -1,7 +1,7 @@
 from django.test import TestCase
 from django.test.client import Client
 from wagtail.wagtailredirects import models
-from wagtail.tests.utils import login
+from wagtail.tests.utils import WagtailTestUtils
 from django.core.urlresolvers import reverse
 
 
@@ -66,15 +66,17 @@ class TestRedirects(TestCase):
         self.assertTrue(r.has_header('Location'))
 
 
-class TestRedirectsIndexView(TestCase):
+class TestRedirectsIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailredirects_index'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailredirects/index.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -88,9 +90,9 @@ class TestRedirectsIndexView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestRedirectsAddView(TestCase):
+class TestRedirectsAddView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailredirects_add_redirect'), params)
@@ -98,8 +100,10 @@ class TestRedirectsAddView(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailredirects_add_redirect'), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailredirects/add.html')
 
     def test_add(self):
         response = self.post({
@@ -110,6 +114,7 @@ class TestRedirectsAddView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailredirects_index'))
 
         # Check that the redirect was created
         redirects = models.Redirect.objects.filter(old_path='/test')
@@ -127,14 +132,14 @@ class TestRedirectsAddView(TestCase):
         self.assertEqual(response.status_code, 200)
 
 
-class TestRedirectsEditView(TestCase):
+class TestRedirectsEditView(TestCase, WagtailTestUtils):
     def setUp(self):
         # Create a redirect to edit
         self.redirect = models.Redirect(old_path='/test', redirect_link='http://www.test.com/')
         self.redirect.save()
 
         # Login
-        login(self.client)
+        self.login()
 
     def get(self, params={}, redirect_id=None):
         return self.client.get(reverse('wagtailredirects_edit_redirect', args=(redirect_id or self.redirect.id, )), params)
@@ -142,8 +147,10 @@ class TestRedirectsEditView(TestCase):
     def post(self, post_data={}, redirect_id=None):
         return self.client.post(reverse('wagtailredirects_edit_redirect', args=(redirect_id or self.redirect.id, )), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailredirects/edit.html')
 
     def test_nonexistant_redirect(self):
         self.assertEqual(self.get(redirect_id=100000).status_code, 404)
@@ -157,6 +164,7 @@ class TestRedirectsEditView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailredirects_index'))
 
         # Check that the redirect was edited
         redirects = models.Redirect.objects.filter(old_path='/test')
@@ -173,14 +181,14 @@ class TestRedirectsEditView(TestCase):
         # Should not redirect to index
         self.assertEqual(response.status_code, 200)
 
-class TestRedirectsDeleteView(TestCase):
+class TestRedirectsDeleteView(TestCase, WagtailTestUtils):
     def setUp(self):
         # Create a redirect to edit
         self.redirect = models.Redirect(old_path='/test', redirect_link='http://www.test.com/')
         self.redirect.save()
 
         # Login
-        login(self.client)
+        self.login()
 
     def get(self, params={}, redirect_id=None):
         return self.client.get(reverse('wagtailredirects_delete_redirect', args=(redirect_id or self.redirect.id, )), params)
@@ -188,8 +196,10 @@ class TestRedirectsDeleteView(TestCase):
     def post(self, post_data={}, redirect_id=None):
         return self.client.post(reverse('wagtailredirects_delete_redirect', args=(redirect_id or self.redirect.id, )), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailredirects/confirm_delete.html')
 
     def test_nonexistant_redirect(self):
         self.assertEqual(self.get(redirect_id=100000).status_code, 404)
@@ -201,6 +211,7 @@ class TestRedirectsDeleteView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailredirects_index'))
 
         # Check that the redirect was deleted
         redirects = models.Redirect.objects.filter(old_path='/test')

wagtail/wagtailsearch/forms.py

@@ -31,6 +31,8 @@ EditorsPickFormSetBase = inlineformset_factory(models.Query, models.EditorsPick,
 
 
 class EditorsPickFormSet(EditorsPickFormSetBase):
+    minimum_forms = 1
+    minimum_forms_message = _("Please specify at least one recommendation for this search term.")
     def add_fields(self, form, *args, **kwargs):
         super(EditorsPickFormSet, self).add_fields(form, *args, **kwargs)
 
@@ -40,3 +42,20 @@ class EditorsPickFormSet(EditorsPickFormSetBase):
 
         # Remove query field
         del form.fields['query']
+
+    def clean(self):
+        # Editors pick must have at least one recommended page to be valid
+        # Check there is at least one non-deleted form.
+        non_deleted_forms = self.total_form_count()
+        non_empty_forms = 0
+        for i in xrange(0, self.total_form_count()):
+            form = self.forms[i]
+            if self.can_delete and self._should_delete_form(form):
+                non_deleted_forms -= 1
+            if not (form.instance.id is None and not form.has_changed()):
+                non_empty_forms += 1
+        if (
+            non_deleted_forms < self.minimum_forms
+            or non_empty_forms < self.minimum_forms
+        ):
+            raise forms.ValidationError(self.minimum_forms_message)

wagtail/wagtailsearch/models.py

@@ -5,9 +5,10 @@ from indexed import Indexed
 import datetime
 import string
 
+MAX_QUERY_STRING_LENGTH = 255
 
 class Query(models.Model):
-    query_string = models.CharField(max_length=255, unique=True)
+    query_string = models.CharField(max_length=MAX_QUERY_STRING_LENGTH, unique=True)
 
     def save(self, *args, **kwargs):
         # Normalise query string
@@ -48,6 +49,9 @@ class Query(models.Model):
 
     @staticmethod
     def normalise_query_string(query_string):
+        # Truncate query string
+        if len(query_string) > MAX_QUERY_STRING_LENGTH:
+            query_string = query_string[:MAX_QUERY_STRING_LENGTH]
         # Convert query_string to lowercase
         query_string = query_string.lower()
 

wagtail/wagtailsearch/templates/wagtailsearch/editorspicks/add.html

@@ -6,7 +6,7 @@
     {% include "wagtailadmin/shared/header.html" with title=add_str icon="pick" %}
 
     <div class="nice-padding">
-        {% blocktrans %}s
+        {% blocktrans %}
         <p>Editors picks are a means of recommending specific pages that might not organically come high up in search results. E.g recommending your primary donation page to a user searching with a less common term like "<em>giving</em>".</p>
         {% endblocktrans %}
         {% blocktrans %}

wagtail/wagtailsearch/tests/test_editorspicks.py

@@ -1,5 +1,5 @@
 from django.test import TestCase
-from wagtail.tests.utils import login
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.wagtailsearch import models
 
 
@@ -45,15 +45,17 @@ class TestEditorsPicks(TestCase):
         self.assertEqual(models.Query.get("root page").editors_picks.last().description, "Last editors pick")
 
 
-class TestEditorsPicksIndexView(TestCase):
+class TestEditorsPicksIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get('/admin/search/editorspicks/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/index.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -67,20 +69,24 @@ class TestEditorsPicksIndexView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestEditorsPicksAddView(TestCase):
+class TestEditorsPicksAddView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get('/admin/search/editorspicks/add/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/add.html')
+
+    # TODO: Test posting
 
 
-class TestEditorsPicksEditView(TestCase):
+class TestEditorsPicksEditView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create an editors pick to edit
         self.query = models.Query.get("Hello")
@@ -89,13 +95,17 @@ class TestEditorsPicksEditView(TestCase):
     def get(self, params={}):
         return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/edit.html')
+
+    # TODO: Test posting
 
 
-class TestEditorsPicksDeleteView(TestCase):
+class TestEditorsPicksDeleteView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
         # Create an editors pick to delete
         self.query = models.Query.get("Hello")
@@ -104,5 +114,9 @@ class TestEditorsPicksDeleteView(TestCase):
     def get(self, params={}):
         return self.client.get('/admin/search/editorspicks/' + str(self.query.id) + '/delete/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/editorspicks/confirm_delete.html')
+
+    # TODO: Test posting

wagtail/wagtailsearch/tests/test_frontend.py

@@ -5,8 +5,10 @@ class TestSearchView(TestCase):
     def get(self, params={}):
         return self.client.get('/search/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/search_results.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -24,8 +26,10 @@ class TestSuggestionsView(TestCase):
     def get(self, params={}):
         return self.client.get('/search/suggest/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        # TODO: Check that a valid JSON document was returned
 
     def test_search(self):
         response = self.get({'q': "Hello"})

wagtail/wagtailsearch/tests/test_queries.py

@@ -1,7 +1,7 @@
 from django.test import TestCase
 from django.core import management
 from wagtail.wagtailsearch import models
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from StringIO import StringIO
 
 
@@ -53,6 +53,16 @@ class TestQueryStringNormalisation(TestCase):
         for query in queries:
             self.assertNotEqual(self.query, models.Query.get(query))
 
+    def test_truncation(self):
+        test_querystring = 'a' * 1000
+        result = models.Query.normalise_query_string(test_querystring)
+        self.assertEqual(len(result), 255)
+
+    def test_no_truncation(self):
+        test_querystring = 'a' * 10
+        result = models.Query.normalise_query_string(test_querystring)
+        self.assertEqual(len(result), 10)
+
 
 class TestQueryPopularity(TestCase):
     def test_query_popularity(self):
@@ -139,15 +149,18 @@ class TestGarbageCollectCommand(TestCase):
     # TODO: Test that this command is acctually doing its job
 
 
-class TestQueryChooserView(TestCase):
+class TestQueryChooserView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get('/admin/search/queries/chooser/', params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsearch/queries/chooser/chooser.html')
+        self.assertTemplateUsed(response, 'wagtailsearch/queries/chooser/chooser.js')
 
     def test_search(self):
         response = self.get({'q': "Hello"})

wagtail/wagtailsearch/views/editorspicks.py

@@ -45,12 +45,12 @@ def index(request):
 
 
 def save_editorspicks(query, new_query, editors_pick_formset):
-    # Set sort_order
-    for i, form in enumerate(editors_pick_formset.ordered_forms):
-        form.instance.sort_order = i
-
     # Save
     if editors_pick_formset.is_valid():
+        # Set sort_order
+        for i, form in enumerate(editors_pick_formset.ordered_forms):
+            form.instance.sort_order = i
+
         editors_pick_formset.save()
 
         # If query was changed, move all editors picks to the new query
@@ -72,10 +72,14 @@ def add(request):
 
             # Save editors picks
             editors_pick_formset = forms.EditorsPickFormSet(request.POST, instance=query)
-
             if save_editorspicks(query, query, editors_pick_formset):
                 messages.success(request, _("Editor's picks for '{0}' created.").format(query))
                 return redirect('wagtailsearch_editorspicks_index')
+            else:
+                if len(editors_pick_formset.non_form_errors()):
+                    messages.error(request, " ".join(error for error in editors_pick_formset.non_form_errors()))  # formset level error (e.g. no forms submitted)
+                else:
+                    messages.error(request, _("Recommendations have not been created due to errors"))  # specific errors will be displayed within form fields
         else:
             editors_pick_formset = forms.EditorsPickFormSet()
     else:
@@ -95,15 +99,22 @@ def edit(request, query_id):
     if request.POST:
         # Get query
         query_form = forms.QueryForm(request.POST)
+        # and the recommendations
+        editors_pick_formset = forms.EditorsPickFormSet(request.POST, instance=query)
+
         if query_form.is_valid():
             new_query = models.Query.get(query_form['query_string'].value())
 
             # Save editors picks
-            editors_pick_formset = forms.EditorsPickFormSet(request.POST, instance=query)
-
             if save_editorspicks(query, new_query, editors_pick_formset):
                 messages.success(request, _("Editor's picks for '{0}' updated.").format(new_query))
                 return redirect('wagtailsearch_editorspicks_index')
+            else:
+                if len(editors_pick_formset.non_form_errors()):
+                    messages.error(request, " ".join(error for error in editors_pick_formset.non_form_errors()))  # formset level error (e.g. no forms submitted)
+                else:
+                    messages.error(request, _("Recommendations have not been saved due to errors"))  # specific errors will be displayed within form fields
+
     else:
         query_form = forms.QueryForm(initial=dict(query_string=query.query_string))
         editors_pick_formset = forms.EditorsPickFormSet(instance=query)

wagtail/wagtailsnippets/tests.py

@@ -2,46 +2,50 @@ from django.test import TestCase
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
 
-from wagtail.tests.utils import login, unittest
+from wagtail.tests.utils import unittest, WagtailTestUtils
 from wagtail.tests.models import Advert, AlphaSnippet, ZuluSnippet
 from wagtail.wagtailsnippets.models import register_snippet, SNIPPET_MODELS
 
 from wagtail.wagtailsnippets.views.snippets import get_content_type_from_url_params, get_snippet_edit_handler
 from wagtail.wagtailsnippets.edit_handlers import SnippetChooserPanel
 
-class TestSnippetIndexView(TestCase):
+class TestSnippetIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailsnippets_index'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsnippets/snippets/index.html')
 
     def test_displays_snippet(self):
         self.assertContains(self.get(), "Adverts")
 
 
-class TestSnippetListView(TestCase):
+class TestSnippetListView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailsnippets_list',
                                        args=('tests', 'advert')),
                                params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsnippets/snippets/type_index.html')
 
     def test_displays_add_button(self):
         self.assertContains(self.get(), "Add advert")
 
 
-class TestSnippetCreateView(TestCase):
+class TestSnippetCreateView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailsnippets_create',
@@ -53,8 +57,10 @@ class TestSnippetCreateView(TestCase):
                                args=('tests', 'advert')),
                                post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsnippets/snippets/create.html')
 
     def test_create_invalid(self):
         response = self.post(post_data={'foo': 'bar'})
@@ -65,20 +71,21 @@ class TestSnippetCreateView(TestCase):
         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')))
 
         snippets = Advert.objects.filter(text='test_advert')
         self.assertEqual(snippets.count(), 1)
         self.assertEqual(snippets.first().url, 'http://www.example.com/')
 
 
-class TestSnippetEditView(TestCase):
+class TestSnippetEditView(TestCase, WagtailTestUtils):
     def setUp(self):
         self.test_snippet = Advert()
         self.test_snippet.text = 'test_advert'
         self.test_snippet.url = 'http://www.example.com/'
         self.test_snippet.save()
 
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailsnippets_edit',
@@ -90,8 +97,10 @@ class TestSnippetEditView(TestCase):
                                         args=('tests', 'advert', self.test_snippet.id)),
                                 post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailsnippets/snippets/edit.html')
 
     def test_non_existant_model(self):
         response = self.client.get(reverse('wagtailsnippets_edit',
@@ -112,20 +121,21 @@ class TestSnippetEditView(TestCase):
         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')))
 
         snippets = Advert.objects.filter(text='edited_test_advert')
         self.assertEqual(snippets.count(), 1)
         self.assertEqual(snippets.first().url, 'http://www.example.com/edited')
 
 
-class TestSnippetDelete(TestCase):
+class TestSnippetDelete(TestCase, WagtailTestUtils):
     def setUp(self):
         self.test_snippet = Advert()
         self.test_snippet.text = 'test_advert'
         self.test_snippet.url = 'http://www.example.com/'
         self.test_snippet.save()
 
-        login(self.client)
+        self.login()
 
     def test_delete_get(self):
         response = self.client.get(reverse('wagtailsnippets_delete', args=('tests', 'advert', self.test_snippet.id, )))
@@ -137,6 +147,7 @@ class TestSnippetDelete(TestCase):
 
         # Should be redirected to explorer page
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailsnippets_list', args=('tests', 'advert')))
 
         # Check that the page is gone
         self.assertEqual(Advert.objects.filter(text='test_advert').count(), 0)

wagtail/wagtailusers/tests.py

@@ -1,18 +1,20 @@
 from django.test import TestCase
 from django.core.urlresolvers import reverse
 from django.contrib.auth.models import User
-from wagtail.tests.utils import login
+from wagtail.tests.utils import WagtailTestUtils
 
 
-class TestUserIndexView(TestCase):
+class TestUserIndexView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailusers_index'), params)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/index.html')
 
     def test_search(self):
         response = self.get({'q': "Hello"})
@@ -26,9 +28,9 @@ class TestUserIndexView(TestCase):
             self.assertEqual(response.status_code, 200)
 
 
-class TestUserCreateView(TestCase):
+class TestUserCreateView(TestCase, WagtailTestUtils):
     def setUp(self):
-        login(self.client)
+        self.login()
 
     def get(self, params={}):
         return self.client.get(reverse('wagtailusers_create'), params)
@@ -36,8 +38,10 @@ class TestUserCreateView(TestCase):
     def post(self, post_data={}):
         return self.client.post(reverse('wagtailusers_create'), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/create.html')
 
     def test_create(self):
         response = self.post({
@@ -51,6 +55,7 @@ class TestUserCreateView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailusers_index'))
 
         # Check that the user was created
         users = User.objects.filter(username='testuser')
@@ -58,13 +63,13 @@ class TestUserCreateView(TestCase):
         self.assertEqual(users.first().email, 'test@user.com')
 
 
-class TestUserEditView(TestCase):
+class TestUserEditView(TestCase, WagtailTestUtils):
     def setUp(self):
         # Create a user to edit
         self.test_user = User.objects.create_user(username='testuser', email='testuser@email.com', password='password')
 
         # Login
-        login(self.client)
+        self.login()
 
     def get(self, params={}, user_id=None):
         return self.client.get(reverse('wagtailusers_edit', args=(user_id or self.test_user.id, )), params)
@@ -72,8 +77,10 @@ class TestUserEditView(TestCase):
     def post(self, post_data={}, user_id=None):
         return self.client.post(reverse('wagtailusers_edit', args=(user_id or self.test_user.id, )), post_data)
 
-    def test_status_code(self):
-        self.assertEqual(self.get().status_code, 200)
+    def test_simple(self):
+        response = self.get()
+        self.assertEqual(response.status_code, 200)
+        self.assertTemplateUsed(response, 'wagtailusers/edit.html')
 
     def test_nonexistant_redirect(self):
         self.assertEqual(self.get(user_id=100000).status_code, 404)
@@ -90,6 +97,7 @@ class TestUserEditView(TestCase):
 
         # Should redirect back to index
         self.assertEqual(response.status_code, 302)
+        self.assertURLEqual(response.url, reverse('wagtailusers_index'))
 
         # Check that the user was edited
         user = User.objects.get(id=self.test_user.id)