Merge pull request #419 from gasman/serve_preview

Use serve_preview() rather than serve() for all preview / moderation / view_draft actions

docs/building_your_site/djangodevelopers.rst

@@ -201,6 +201,7 @@ Properties:
 * status_string
 * subpage_types
 * indexed_fields
+* preview_modes
 
 Methods:
 
@@ -213,8 +214,7 @@ Methods:
 * get_descendants
 * get_siblings
 * search
-* get_page_modes
-* show_as_mode
+* serve_preview
 
 
 Page Queryset Methods

docs/model_recipes.rst

@@ -190,11 +190,11 @@ Load Alternate Templates by Overriding get_template()
 
 
 
-Page Modes
-----------
+Preview Modes
+-------------
 
-get_page_modes
-show_as_mode
+preview_modes
+serve_preview
 
 
 

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

@@ -39,12 +39,12 @@
 
                 <li class="actions preview">
                     {% trans 'Preview' as preview_label %}
-                    {% if display_modes|length > 1 %}
+                    {% if preview_modes|length > 1 %}
                         <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">
-                                {% for mode_name, mode_display_name in display_modes %}
+                                {% for mode_name, mode_display_name in preview_modes %}
                                     <li>
                                         {% include "wagtailadmin/pages/_preview_button_on_create.html" with mode=mode_name label=mode_display_name %}
                                     </li>

wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html

@@ -47,12 +47,12 @@
 
                 <li class="actions preview">
                     {% trans 'Preview' as preview_label %}
-                    {% if display_modes|length > 1 %}
+                    {% if preview_modes|length > 1 %}
                         <div class="dropdown dropup dropdown-button match-width">
                             {% include "wagtailadmin/pages/_preview_button_on_edit.html" with label=preview_label icon=1 %}
                             <div class="dropdown-toggle icon icon-arrow-up"></div>
                             <ul role="menu">
-                                {% for mode_name, mode_display_name in display_modes %}
+                                {% for mode_name, mode_display_name in preview_modes %}
                                     <li>
                                         {% include "wagtailadmin/pages/_preview_button_on_edit.html" with mode=mode_name label=mode_display_name %}
                                     </li>

wagtail/wagtailadmin/views/pages.py

@@ -1,3 +1,5 @@
+import warnings
+
 from django.http import Http404, HttpResponse
 from django.shortcuts import render, redirect, get_object_or_404
 from django.core.exceptions import ValidationError, PermissionDenied
@@ -7,6 +9,7 @@ from django.contrib.auth.decorators import permission_required
 from django.core.paginator import Paginator, EmptyPage, PageNotAnInteger
 from django.utils import timezone
 from django.utils.translation import ugettext as _
+from django.views.decorators.http import require_GET
 from django.views.decorators.vary import vary_on_headers
 
 from wagtail.wagtailadmin.edit_handlers import TabbedInterface, ObjectList
@@ -230,7 +233,7 @@ def create(request, content_type_app_name, content_type_model_name, parent_page_
         'page_class': page_class,
         'parent_page': parent_page,
         'edit_handler': edit_handler,
-        'display_modes': page.get_page_modes(),
+        'preview_modes': page.preview_modes,
         'form': form, # Used in unit tests
     })
 
@@ -361,7 +364,7 @@ def edit(request, page_id):
         'page': page,
         'edit_handler': edit_handler,
         'errors_debug': errors_debug,
-        'display_modes': page.get_page_modes(),
+        'preview_modes': page.preview_modes,
         'form': form, # Used in unit tests
     })
 
@@ -393,7 +396,28 @@ def delete(request, page_id):
 @permission_required('wagtailadmin.access_admin')
 def view_draft(request, page_id):
     page = get_object_or_404(Page, id=page_id).get_latest_revision_as_page()
-    return page.serve(request)
+    return page.serve_preview(page.dummy_request(), page.default_preview_mode)
+
+
+def get_preview_response(page, preview_mode):
+    """
+    Helper function for preview_on_edit and preview_on_create -
+    return a page's preview response via either serve_preview or the deprecated
+    show_as_mode method
+    """
+    # Check the deprecated Page.show_as_mode method, as subclasses of Page
+    # might be overriding that to return a response
+    response = page.show_as_mode(preview_mode)
+    if response:
+        warnings.warn(
+            "Defining 'show_as_mode' on a page model is deprecated. Use 'serve_preview' instead",
+            DeprecationWarning
+        )
+        return response
+    else:
+        # show_as_mode did not return a response, so go ahead and use the 'proper'
+        # serve_preview method
+        return page.serve_preview(page.dummy_request(), preview_mode)
 
 
 @permission_required('wagtailadmin.access_admin')
@@ -409,19 +433,8 @@ def preview_on_edit(request, page_id):
     if form.is_valid():
         form.save(commit=False)
 
-        # 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
-        # failure - as such, we strip out the X-Requested-With header to get Django to return
-        # an HTML error response
-        request.META.pop('HTTP_X_REQUESTED_WITH', None)
-
-        try:
-            display_mode = request.GET['mode']
-        except KeyError:
-            display_mode = page.get_page_modes()[0][0]
-
-        response = page.show_as_mode(display_mode)
+        preview_mode = request.GET.get('mode', page.default_preview_mode)
+        response = get_preview_response(page, preview_mode)
 
         response['X-Wagtail-Preview'] = 'ok'
         return response
@@ -432,7 +445,7 @@ def preview_on_edit(request, page_id):
         response = render(request, 'wagtailadmin/pages/edit.html', {
             'page': page,
             'edit_handler': edit_handler,
-            'display_modes': page.get_page_modes(),
+            'preview_modes': page.preview_modes,
         })
         response['X-Wagtail-Preview'] = 'error'
         return response
@@ -465,18 +478,8 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p
         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
-        # failure - as such, we strip out the X-Requested-With header to get Django to return
-        # an HTML error response
-        request.META.pop('HTTP_X_REQUESTED_WITH', None)
-
-        try:
-            display_mode = request.GET['mode']
-        except KeyError:
-            display_mode = page.get_page_modes()[0][0]
-        response = page.show_as_mode(display_mode)
+        preview_mode = request.GET.get('mode', page.default_preview_mode)
+        response = get_preview_response(page, preview_mode)
 
         response['X-Wagtail-Preview'] = 'ok'
         return response
@@ -490,7 +493,7 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p
             'page_class': page_class,
             'parent_page': parent_page,
             'edit_handler': edit_handler,
-            'display_modes': page.get_page_modes(),
+            'preview_modes': page.preview_modes,
         })
         response['X-Wagtail-Preview'] = 'error'
         return response
@@ -727,6 +730,7 @@ def reject_moderation(request, revision_id):
 
 
 @permission_required('wagtailadmin.access_admin')
+@require_GET
 def preview_for_moderation(request, revision_id):
     revision = get_object_or_404(PageRevision, id=revision_id)
     if not revision.page.permissions_for_user(request.user).can_publish():
@@ -740,4 +744,6 @@ def preview_for_moderation(request, revision_id):
 
     request.revision_id = revision_id
 
-    return page.serve(request)
+    # pass in the real user request rather than page.dummy_request(), so that request.user
+    # and request.revision_id will be picked up by the wagtail user bar
+    return page.serve_preview(request, page.default_preview_mode)

wagtail/wagtailcore/models.py

@@ -706,25 +706,64 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)):
                                 "request middleware returned a response")
         return request
 
-    def get_page_modes(self):
+    DEFAULT_PREVIEW_MODES = [('', 'Default')]
+
+    @property
+    def preview_modes(self):
         """
-        Return a list of (internal_name, display_name) tuples for the modes in which
+        A list of (internal_name, display_name) tuples for the modes in which
         this page can be displayed for preview/moderation purposes. Ordinarily a page
         will only have one display mode, but subclasses of Page can override this -
         for example, a page containing a form might have a default view of the form,
         and a post-submission 'thankyou' page
         """
-        return [('', 'Default')]
+        modes = self.get_page_modes()
+        if modes is not Page.DEFAULT_PREVIEW_MODES:
+            # User has overriden get_page_modes instead of using preview_modes
+            warnings.warn("Overriding get_page_modes is deprecated. Define a preview_modes property instead", DeprecationWarning)
+
+        return modes
+
+    def get_page_modes(self):
+        # Deprecated accessor for the preview_modes property
+        return Page.DEFAULT_PREVIEW_MODES
+
+    @property
+    def default_preview_mode(self):
+        return self.preview_modes[0][0]
+
+    def serve_preview(self, request, mode_name):
+        """
+        Return an HTTP response for use in page previews. Normally this would be equivalent
+        to self.serve(request), since we obviously want the preview to be indicative of how
+        it looks on the live site. However, there are a couple of cases where this is not
+        appropriate, and custom behaviour is required:
+
+        1) The page has custom routing logic that derives some additional required
+        args/kwargs to be passed to serve(). The routing mechanism is bypassed when
+        previewing, so there's no way to know what args we should pass. In such a case,
+        the page model needs to implement its own version of serve_preview.
+
+        2) The page has several different renderings that we would like to be able to see
+        when previewing - for example, a form page might have one rendering that displays
+        the form, and another rendering to display a landing page when the form is posted.
+        This can be done by setting a custom preview_modes list on the page model -
+        Wagtail will allow the user to specify one of those modes when previewing, and
+        pass the chosen mode_name to serve_preview so that the page model can decide how
+        to render it appropriately. (Page models that do not specify their own preview_modes
+        list will always receive an empty string as mode_name.)
+
+        Any templates rendered during this process should use the 'request' object passed
+        here - this ensures that request.user and other properties are set appropriately for
+        the wagtail user bar to be displayed. This request will always be a GET.
+        """
+        return self.serve(request)
 
     def show_as_mode(self, mode_name):
-        """
-        Given an internal name from the get_page_modes() list, return an HTTP response
-        indicative of the page being viewed in that mode. By default this passes a
-        dummy request into the serve() mechanism, ensuring that it matches the behaviour
-        on the front-end; subclasses that define additional page modes will need to
-        implement alternative logic to serve up the appropriate view here.
-        """
-        return self.serve(self.dummy_request())
+        # Deprecated API for rendering previews. If this returns something other than None,
+        # we know that a subclass of Page has overridden this, and we should try to work with
+        # that response if possible.
+        return None
 
     def get_cached_paths(self):
         """

wagtail/wagtailforms/models.py

@@ -170,19 +170,18 @@ class AbstractForm(Page):
             'form': form,
         })
 
-    def get_page_modes(self):
-        return [
-            ('form', 'Form'),
-            ('landing', 'Landing page'),
-        ]
+    preview_modes = [
+        ('form', 'Form'),
+        ('landing', 'Landing page'),
+    ]
 
-    def show_as_mode(self, mode):
+    def serve_preview(self, request, mode):
         if mode == 'landing':
-            return render(self.dummy_request(), self.landing_page_template, {
+            return render(request, self.landing_page_template, {
                 'self': self,
             })
         else:
-            return super(AbstractForm, self).show_as_mode(mode)
+            return super(AbstractForm, self).serve_preview(request, mode)
 
 
 class AbstractEmailForm(AbstractForm):

wagtail/wagtailforms/tests.py

@@ -61,7 +61,7 @@ class TestPageModes(TestCase):
         self.form_page = Page.objects.get(url_path='/home/contact-us/').specific
 
     def test_form(self):
-        response = self.form_page.show_as_mode('form')
+        response = self.form_page.serve_preview(self.form_page.dummy_request(), 'form')
 
         # Check response
         self.assertContains(response, """<label for="id_your-email">Your email</label>""")
@@ -69,7 +69,7 @@ class TestPageModes(TestCase):
         self.assertTemplateNotUsed(response, 'tests/form_page_landing.html')
 
     def test_landing(self):
-        response = self.form_page.show_as_mode('landing')
+        response = self.form_page.serve_preview(self.form_page.dummy_request(), 'landing')
 
         # Check response
         self.assertContains(response, "Thank you for your feedback.")