Update Page.route API so that it returns a Page instance rather than an HttpResponse; catch the latter in the 'serve' view for backward compatibility

wagtail/tests/fixtures/test.json

@@ -23,7 +23,7 @@
     "model": "wagtailcore.page",
     "fields": {
         "title": "Welcome to the Wagtail test site!",
-        "numchild": 3,
+        "numchild": 4,
         "show_in_menus": false,
         "live": true,
         "depth": 2,
@@ -257,6 +257,29 @@
     }
 },
 
+{
+    "pk": 10,
+    "model": "wagtailcore.page",
+    "fields": {
+        "title": "Old style route method",
+        "numchild": 0,
+        "show_in_menus": true,
+        "live": true,
+        "depth": 3,
+        "content_type": ["tests", "pagewitholdstyleroutemethod"],
+        "path": "000100010004",
+        "url_path": "/home/old-style-route/",
+        "slug": "old-style-route"
+    }
+},
+{
+    "pk": 10,
+    "model": "tests.pagewitholdstyleroutemethod",
+    "fields": {
+        "content": "<p>Test with old style route method</p>"
+    }
+},
+
 {
     "pk": 1,
     "model": "wagtailcore.site",

wagtail/tests/models.py

@@ -106,6 +106,19 @@ class SimplePage(Page):
     content = models.TextField()
 
 
+class PageWithOldStyleRouteMethod(Page):
+    """
+    Prior to Wagtail 0.4, the route() method on Page returned an HttpResponse
+    rather than a Page instance. As subclasses of Page may override route,
+    we need to continue accepting this convention (albeit as a deprecated API).
+    """
+    content = models.TextField()
+    template = 'tests/simple_page.html'
+
+    def route(self, request, path_components):
+        return self.serve(request)
+
+
 # Event page
 
 class EventPageCarouselItem(Orderable, CarouselItem):

wagtail/wagtailcore/models.py

@@ -415,7 +415,7 @@ class Page(six.with_metaclass(PageBase, MP_Node, ClusterableModel, Indexed)):
         else:
             # request is for this very page
             if self.live:
-                return self.serve(request)
+                return self
             else:
                 raise Http404
 

wagtail/wagtailcore/tests/test_page_model.py

@@ -2,7 +2,7 @@ from django.test import TestCase, Client
 from django.http import HttpRequest, Http404
 
 from wagtail.wagtailcore.models import Page, Site
-from wagtail.tests.models import EventPage, EventIndex, SimplePage
+from wagtail.tests.models import EventPage, EventIndex, SimplePage, PageWithOldStyleRouteMethod
 
 
 class TestSiteRouting(TestCase):
@@ -136,8 +136,13 @@ class TestRouting(TestCase):
 
         request = HttpRequest()
         request.path = '/events/christmas/'
-        response = homepage.route(request, ['events', 'christmas'])
+        found_page = homepage.route(request, ['events', 'christmas'])
+        self.assertEqual(found_page, christmas_page)
 
+    def test_request_serving(self):
+        christmas_page = EventPage.objects.get(url_path='/home/events/christmas/')
+        request = HttpRequest()
+        response = christmas_page.serve(request)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context_data['self'], christmas_page)
         used_template = response.resolve_template(response.template_name)
@@ -226,6 +231,18 @@ class TestServeView(TestCase):
         self.assertContains(response, '<a href="/events/christmas/">Christmas</a>')
 
 
+    def test_old_style_routing(self):
+        """
+        Test that route() methods that return an HttpResponse are correctly handled
+        """
+        response = self.client.get('/old-style-route/')
+        expected_page = PageWithOldStyleRouteMethod.objects.get(url_path='/home/old-style-route/')
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.context['self'], expected_page)
+        self.assertEqual(response.templates[0].name, 'tests/simple_page.html')
+
+
 class TestStaticSitePaths(TestCase):
     def setUp(self):
         self.root_page = Page.objects.get(id=1)

wagtail/wagtailcore/views.py

@@ -1,4 +1,6 @@
-from django.http import Http404
+import warnings
+
+from django.http import HttpResponse, Http404
 
 
 def serve(request, path):
@@ -8,4 +10,12 @@ def serve(request, path):
         raise Http404
 
     path_components = [component for component in path.split('/') if component]
-    return request.site.root_page.specific.route(request, path_components)
+    page = request.site.root_page.specific.route(request, path_components)
+    if isinstance(page, HttpResponse):
+        warnings.warn(
+            "Page.route should return a Page, not an HttpResponse",
+            DeprecationWarning
+        )
+        return page
+
+    return page.serve(request)