From f0aee82906621f7989a1e88ed2f645cf5c80ec75 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 7 Apr 2014 20:08:33 +0100 Subject: [PATCH 1/5] Add a dummy_request method to Page so that we can generate page previews without passing in the active request object (which may have undesirable side effects, e.g. may be a POST) --- wagtail/wagtailadmin/views/pages.py | 24 +++++++++++------- wagtail/wagtailcore/models.py | 39 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 3fb905715..4c50eaafe 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -340,12 +340,14 @@ def preview_on_edit(request, page_id): if form.is_valid(): form.save(commit=False) - # FIXME: passing the original request to page.serve is dodgy (particularly if page.serve has - # special treatment of POSTs). Ought to construct one that more or less matches what would be sent - # as a front-end GET request + # 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) - request.META.pop('HTTP_X_REQUESTED_WITH', None) # Make this request appear to the page's serve method as a non-ajax one, as they will often implement custom behaviour for XHR - response = page.serve(request) + response = page.serve(page.dummy_request()) response['X-Wagtail-Preview'] = 'ok' return response @@ -380,10 +382,14 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p if form.is_valid(): form.save(commit=False) - # FIXME: passing the original request to page.serve is dodgy (particularly if page.serve has - # special treatment of POSTs). Ought to construct one that more or less matches what would be sent - # as a front-end GET request - response = page.serve(request) + # 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) + + response = page.serve(page.dummy_request()) response['X-Wagtail-Preview'] = 'ok' return response diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index b06137b9d..5c3c50f9b 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -1,5 +1,7 @@ import sys import os +from StringIO import StringIO +from urlparse import urlparse from modelcluster.models import ClusterableModel @@ -7,6 +9,8 @@ from django.db import models, connection, transaction from django.db.models import get_model, Q from django.http import Http404 from django.core.cache import cache +from django.core.handlers.wsgi import WSGIRequest +from django.core.handlers.base import BaseHandler from django.contrib.contenttypes.models import ContentType from django.contrib.auth.models import Group from django.conf import settings @@ -501,6 +505,41 @@ class Page(MP_Node, ClusterableModel, Indexed): user_perms = UserPagePermissionsProxy(user) return user_perms.for_page(self) + def dummy_request(self): + """ + Construct a HttpRequest object that is, as far as possible, representative of ones that would + receive this page as a response. Used for previewing / moderation and any other place where we + want to display a view of this page in the admin interface without going through the regular + page routing logic. + """ + url = self.full_url + if url: + url_info = urlparse(url) + hostname = url_info.netloc + path = url_info.path + port = url_info.port or 80 + else: + hostname = 'example.com' + path = '/' + port = 80 + + request = WSGIRequest({ + 'REQUEST_METHOD': 'GET', + 'PATH_INFO': path, + 'SERVER_NAME': hostname, + 'SERVER_PORT': port, + 'wsgi.input': StringIO(), + }) + + # Apply middleware to the request - see http://www.mellowmorning.com/2011/04/18/mock-django-request-for-testing/ + handler = BaseHandler() + handler.load_middleware() + for middleware_method in handler._request_middleware: + if middleware_method(request): + raise Exception("Couldn't create request mock object - " + "request middleware returned a response") + return request + def get_navigation_menu_items(): # Get all pages that appear in the navigation menu: ones which have children, From d6cab0a10e02bd74c38fcaa1266bf9886eb7b7f9 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 8 Apr 2014 11:36:08 +0100 Subject: [PATCH 2/5] Add get_page_modes and show_as_mode methods to Page. This abstracts out from the preview methods the details of how the page is generated. --- wagtail/wagtailadmin/views/pages.py | 8 ++++---- wagtail/wagtailcore/models.py | 20 ++++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index 4c50eaafe..a29477813 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -1,8 +1,6 @@ from django.http import Http404, HttpResponse from django.shortcuts import render, redirect, get_object_or_404 from django.core.exceptions import ValidationError, PermissionDenied -from django.template.loader import render_to_string -from django.template import RequestContext from django.contrib import messages from django.contrib.contenttypes.models import ContentType from django.contrib.auth.decorators import permission_required @@ -347,7 +345,8 @@ def preview_on_edit(request, page_id): # an HTML error response request.META.pop('HTTP_X_REQUESTED_WITH', None) - response = page.serve(page.dummy_request()) + display_mode = page.get_page_modes()[0] + response = page.show_as_mode(display_mode) response['X-Wagtail-Preview'] = 'ok' return response @@ -389,7 +388,8 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p # an HTML error response request.META.pop('HTTP_X_REQUESTED_WITH', None) - response = page.serve(page.dummy_request()) + display_mode = page.get_page_modes()[0] + response = page.show_as_mode(display_mode) response['X-Wagtail-Preview'] = 'ok' return response diff --git a/wagtail/wagtailcore/models.py b/wagtail/wagtailcore/models.py index 5c3c50f9b..ee5d50a43 100644 --- a/wagtail/wagtailcore/models.py +++ b/wagtail/wagtailcore/models.py @@ -540,6 +540,26 @@ class Page(MP_Node, ClusterableModel, Indexed): "request middleware returned a response") return request + def get_page_modes(self): + """ + Return 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')] + + 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()) + def get_navigation_menu_items(): # Get all pages that appear in the navigation menu: ones which have children, From 75b127a07c5615a5d1a245b4e032a3488d585bef Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Tue, 8 Apr 2014 16:14:30 +0100 Subject: [PATCH 3/5] Support within wagtailadmin for selecting a view mode in which to preview a page. The 'preview' option is split out from the 'save' popup menu into its own button (it was deemed appropriate to split it off since it doesn't perform any database-level actions). If the page defines multiple page modes in get_page_modes, this button becomes a popup, allowing the editor to choose a mode. Finally, preview_on_create and preview_on_edit are updated to pass the chosen mode into the page's show_as_mode method. --- .../static/wagtailadmin/js/page-editor.js | 2 +- .../pages/_preview_button_on_create.html | 4 ++++ .../pages/_preview_button_on_edit.html | 4 ++++ .../templates/wagtailadmin/pages/create.html | 20 ++++++++++++++++++- .../templates/wagtailadmin/pages/edit.html | 20 +++++++++++++++++-- wagtail/wagtailadmin/views/pages.py | 15 ++++++++++++-- 6 files changed, 59 insertions(+), 6 deletions(-) create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_create.html create mode 100644 wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_edit.html diff --git a/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js b/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js index e34bbaf60..4a2dd4d65 100644 --- a/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js +++ b/wagtail/wagtailadmin/static/wagtailadmin/js/page-editor.js @@ -318,7 +318,7 @@ $(function() { }); /* Set up behaviour of preview button */ - $('#action-preview').click(function() { + $('.action-preview').click(function() { var previewWindow = window.open($(this).data('placeholder'), $(this).data('windowname')); $.ajax({ diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_create.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_create.html new file mode 100644 index 000000000..d8d12a9be --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_create.html @@ -0,0 +1,4 @@ + diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_edit.html new file mode 100644 index 000000000..15f35245b --- /dev/null +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/_preview_button_on_edit.html @@ -0,0 +1,4 @@ + diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/create.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/create.html index 1bfc18381..11cb8a572 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/create.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/create.html @@ -21,7 +21,6 @@
    -
    • {% if parent_page_perms.can_publish_subpage %}
    • {% endif %} @@ -29,6 +28,25 @@
+ +
  • + {% trans 'Preview' as preview_label %} + {% if display_modes|length > 1 %} +
    + {% include "wagtailadmin/pages/_preview_button_on_create.html" with label=preview_label %} +
    +
      + {% for mode_name, mode_display_name in display_modes %} +
    • + {% include "wagtailadmin/pages/_preview_button_on_create.html" with mode=mode_name label=mode_display_name %} +
      • + {% endfor %} +
    +
    + {% else %} + {% include "wagtailadmin/pages/_preview_button_on_create.html" with label=preview_label %} + {% endif %} +
    • diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html index c12419563..8a954ba49 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/pages/edit.html @@ -48,13 +48,29 @@
    • {% endif %}
    • -
    • -
  • +
  • + {% trans 'Preview' as preview_label %} + {% if display_modes|length > 1 %} +
    + {% include "wagtailadmin/pages/_preview_button_on_edit.html" with label=preview_label %} +
    +
      + {% for mode_name, mode_display_name in display_modes %} +
    • + {% include "wagtailadmin/pages/_preview_button_on_edit.html" with mode=mode_name label=mode_display_name %} +
      • + {% endfor %} +
    +
    + {% else %} + {% include "wagtailadmin/pages/_preview_button_on_edit.html" with label=preview_label %} + {% endif %}
    • +

  • {% if page.get_latest_revision %} diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index a29477813..c09805def 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -208,6 +208,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(), }) @@ -292,6 +293,7 @@ def edit(request, page_id): 'page': page, 'edit_handler': edit_handler, 'errors_debug': errors_debug, + 'display_modes': page.get_page_modes(), }) @@ -345,7 +347,11 @@ def preview_on_edit(request, page_id): # an HTML error response request.META.pop('HTTP_X_REQUESTED_WITH', None) - display_mode = page.get_page_modes()[0] + try: + display_mode = request.GET['mode'] + except KeyError: + display_mode = page.get_page_modes()[0] + response = page.show_as_mode(display_mode) response['X-Wagtail-Preview'] = 'ok' @@ -357,6 +363,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(), }) response['X-Wagtail-Preview'] = 'error' return response @@ -388,7 +395,10 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p # an HTML error response request.META.pop('HTTP_X_REQUESTED_WITH', None) - display_mode = page.get_page_modes()[0] + try: + display_mode = request.GET['mode'] + except KeyError: + display_mode = page.get_page_modes()[0] response = page.show_as_mode(display_mode) response['X-Wagtail-Preview'] = 'ok' @@ -403,6 +413,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(), }) response['X-Wagtail-Preview'] = 'error' return response From 79334134b79adc1f79acdede3da3a17555a170db Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 28 Apr 2014 15:15:34 +0100 Subject: [PATCH 4/5] Update wagtaildocs, wagtailimages, wagtailsnippets and wagtailusers to register their menu items via the construct_main_menu hook rather than hard-coding them in wagtailadmin_tags --- .../templatetags/wagtailadmin_tags.py | 22 ------------------- wagtail/wagtaildocs/wagtail_hooks.py | 12 ++++++++++ wagtail/wagtailimages/wagtail_hooks.py | 12 ++++++++++ wagtail/wagtailsnippets/wagtail_hooks.py | 13 +++++++++++ wagtail/wagtailusers/wagtail_hooks.py | 12 ++++++++++ 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py index a7aaf38fe..30b4b2b6b 100644 --- a/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py +++ b/wagtail/wagtailadmin/templatetags/wagtailadmin_tags.py @@ -8,8 +8,6 @@ from wagtail.wagtailadmin.menu import MenuItem from wagtail.wagtailcore.models import get_navigation_menu_items, UserPagePermissionsProxy from wagtail.wagtailcore.util import camelcase_to_underscore -from wagtail.wagtailsnippets.permissions import user_can_edit_snippets # TODO: reorganise into pluggable architecture so that wagtailsnippets registers its own menu item - register = template.Library() @@ -46,26 +44,6 @@ def main_nav(context): ] request = context['request'] - user = request.user - - if user.has_perm('wagtailimages.add_image'): - menu_items.append( - MenuItem(_('Images'), urlresolvers.reverse('wagtailimages_index'), classnames='icon icon-image', order=300) - ) - if user.has_perm('wagtaildocs.add_document'): - menu_items.append( - MenuItem(_('Documents'), urlresolvers.reverse('wagtaildocs_index'), classnames='icon icon-doc-full-inverse', order=400) - ) - - if user_can_edit_snippets(user): - menu_items.append( - MenuItem(_('Snippets'), urlresolvers.reverse('wagtailsnippets_index'), classnames='icon icon-snippet', order=500) - ) - - if user.has_module_perms('auth'): - menu_items.append( - MenuItem(_('Users'), urlresolvers.reverse('wagtailusers_index'), classnames='icon icon-user', order=600) - ) for fn in hooks.get_hooks('construct_main_menu'): fn(request, menu_items) diff --git a/wagtail/wagtaildocs/wagtail_hooks.py b/wagtail/wagtaildocs/wagtail_hooks.py index 93d41c9a3..8e8622127 100644 --- a/wagtail/wagtaildocs/wagtail_hooks.py +++ b/wagtail/wagtaildocs/wagtail_hooks.py @@ -1,6 +1,10 @@ from django.conf.urls import include, url +from django.core import urlresolvers +from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin import hooks +from wagtail.wagtailadmin.menu import MenuItem + from wagtail.wagtaildocs import admin_urls @@ -9,3 +13,11 @@ def register_admin_urls(): url(r'^documents/', include(admin_urls)), ] hooks.register('register_admin_urls', register_admin_urls) + + +def construct_main_menu(request, menu_items): + if request.user.has_perm('wagtaildocs.add_document'): + menu_items.append( + MenuItem(_('Documents'), urlresolvers.reverse('wagtaildocs_index'), classnames='icon icon-doc-full-inverse', order=400) + ) +hooks.register('construct_main_menu', construct_main_menu) diff --git a/wagtail/wagtailimages/wagtail_hooks.py b/wagtail/wagtailimages/wagtail_hooks.py index 2309876a5..f14d22904 100644 --- a/wagtail/wagtailimages/wagtail_hooks.py +++ b/wagtail/wagtailimages/wagtail_hooks.py @@ -1,6 +1,10 @@ from django.conf.urls import include, url +from django.core import urlresolvers +from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin import hooks +from wagtail.wagtailadmin.menu import MenuItem + from wagtail.wagtailimages import urls @@ -9,3 +13,11 @@ def register_admin_urls(): url(r'^images/', include(urls)), ] hooks.register('register_admin_urls', register_admin_urls) + + +def construct_main_menu(request, menu_items): + if request.user.has_perm('wagtailimages.add_image'): + menu_items.append( + MenuItem(_('Images'), urlresolvers.reverse('wagtailimages_index'), classnames='icon icon-image', order=300) + ) +hooks.register('construct_main_menu', construct_main_menu) diff --git a/wagtail/wagtailsnippets/wagtail_hooks.py b/wagtail/wagtailsnippets/wagtail_hooks.py index 460cf41dc..35468f7d8 100644 --- a/wagtail/wagtailsnippets/wagtail_hooks.py +++ b/wagtail/wagtailsnippets/wagtail_hooks.py @@ -1,7 +1,12 @@ from django.conf.urls import include, url +from django.core import urlresolvers +from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin import hooks +from wagtail.wagtailadmin.menu import MenuItem + from wagtail.wagtailsnippets import urls +from wagtail.wagtailsnippets.permissions import user_can_edit_snippets def register_admin_urls(): @@ -9,3 +14,11 @@ def register_admin_urls(): url(r'^snippets/', include(urls)), ] hooks.register('register_admin_urls', register_admin_urls) + + +def construct_main_menu(request, menu_items): + if user_can_edit_snippets(request.user): + menu_items.append( + MenuItem(_('Snippets'), urlresolvers.reverse('wagtailsnippets_index'), classnames='icon icon-snippet', order=500) + ) +hooks.register('construct_main_menu', construct_main_menu) diff --git a/wagtail/wagtailusers/wagtail_hooks.py b/wagtail/wagtailusers/wagtail_hooks.py index add9afb2d..d006a4d93 100644 --- a/wagtail/wagtailusers/wagtail_hooks.py +++ b/wagtail/wagtailusers/wagtail_hooks.py @@ -1,6 +1,10 @@ from django.conf.urls import include, url +from django.core import urlresolvers +from django.utils.translation import ugettext_lazy as _ from wagtail.wagtailadmin import hooks +from wagtail.wagtailadmin.menu import MenuItem + from wagtail.wagtailusers import urls @@ -9,3 +13,11 @@ def register_admin_urls(): url(r'^users/', include(urls)), ] hooks.register('register_admin_urls', register_admin_urls) + + +def construct_main_menu(request, menu_items): + if request.user.has_module_perms('auth'): + menu_items.append( + MenuItem(_('Users'), urlresolvers.reverse('wagtailusers_index'), classnames='icon icon-user', order=600) + ) +hooks.register('construct_main_menu', construct_main_menu) From 3dfc58c7836d1cca815d2d6b847a3ee4d1ce4346 Mon Sep 17 00:00:00 2001 From: Matt Westcott Date: Mon, 28 Apr 2014 17:01:08 +0100 Subject: [PATCH 5/5] fix logic for fetching the default page mode - should return the internal name, not the tuple --- wagtail/wagtailadmin/views/pages.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/wagtail/wagtailadmin/views/pages.py b/wagtail/wagtailadmin/views/pages.py index b65aa57c9..00028db22 100644 --- a/wagtail/wagtailadmin/views/pages.py +++ b/wagtail/wagtailadmin/views/pages.py @@ -350,7 +350,7 @@ def preview_on_edit(request, page_id): try: display_mode = request.GET['mode'] except KeyError: - display_mode = page.get_page_modes()[0] + display_mode = page.get_page_modes()[0][0] response = page.show_as_mode(display_mode) @@ -398,7 +398,7 @@ def preview_on_create(request, content_type_app_name, content_type_model_name, p try: display_mode = request.GET['mode'] except KeyError: - display_mode = page.get_page_modes()[0] + display_mode = page.get_page_modes()[0][0] response = page.show_as_mode(display_mode) response['X-Wagtail-Preview'] = 'ok'