From 913607c939b9151c46e9c1ba188fd8b217aa4f81 Mon Sep 17 00:00:00 2001
From: Matt Westcott <matt@west.co.tt>
Date: Fri, 31 Mar 2017 19:30:53 +0100
Subject: [PATCH] Take MRO into account when constructing RoutablePageMixin's
 list of routes

This ensures that routes can be overridden in subclasses, while still respecting the definition order within a class definition.
---
 wagtail/contrib/wagtailroutablepage/models.py | 24 ++++++++++++-------
 wagtail/contrib/wagtailroutablepage/tests.py  |  5 ++++
 wagtail/tests/routablepage/models.py          |  4 ++++
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/wagtail/contrib/wagtailroutablepage/models.py b/wagtail/contrib/wagtailroutablepage/models.py
index d805b0d602..e9b9c3a226 100644
--- a/wagtail/contrib/wagtailroutablepage/models.py
+++ b/wagtail/contrib/wagtailroutablepage/models.py
@@ -49,15 +49,23 @@ class RoutablePageMixin(object):
     @classmethod
     def get_subpage_urls(cls):
         routes = []
-        for attr in dir(cls):
-            val = getattr(cls, attr, None)
-            if hasattr(val, '_routablepage_routes'):
-                routes.extend(val._routablepage_routes)
 
-        return tuple([
-            route[0]
-            for route in reversed(sorted(routes, key=lambda route: route[1]))
-        ])
+        # Loop over this class's defined routes, in method resolution order.
+        # Routes defined in the immediate class take precedence, followed by
+        # immediate superclass and so on
+        for klass in cls.__mro__:
+            routes_for_class = []
+            for val in klass.__dict__.values():
+                if hasattr(val, '_routablepage_routes'):
+                    routes_for_class.extend(val._routablepage_routes)
+
+            # sort routes by _creation_counter so that ones earlier in the class definition
+            # take precedence
+            routes_for_class.sort(key=lambda route: route[1])
+
+            routes.extend(route[0] for route in routes_for_class)
+
+        return tuple(routes)
 
     @classmethod
     def get_resolver(cls):
diff --git a/wagtail/contrib/wagtailroutablepage/tests.py b/wagtail/contrib/wagtailroutablepage/tests.py
index d5191f4323..bd5d48b7be 100644
--- a/wagtail/contrib/wagtailroutablepage/tests.py
+++ b/wagtail/contrib/wagtailroutablepage/tests.py
@@ -110,6 +110,11 @@ class TestRoutablePage(TestCase):
 
         self.assertContains(response, "ARCHIVE BY YEAR: 2014")
 
+    def test_earlier_view_takes_precedence(self):
+        response = self.client.get(self.routable_page.url + 'archive/year/1984/')
+
+        self.assertContains(response, "we were always at war with eastasia")
+
     def test_get_archive_by_author_view(self):
         response = self.client.get(self.routable_page.url + 'archive/author/joe-bloggs/')
 
diff --git a/wagtail/tests/routablepage/models.py b/wagtail/tests/routablepage/models.py
index dd992c9953..daa020765e 100644
--- a/wagtail/tests/routablepage/models.py
+++ b/wagtail/tests/routablepage/models.py
@@ -10,6 +10,10 @@ def routable_page_external_view(request, arg="ARG NOT SET"):
 
 
 class RoutablePageTest(RoutablePage):
+    @route(r'^archive/year/1984/$')
+    def archive_for_1984(self, request):
+        # check that routes are tested in order (and thus this takes precedence over archive_by_year)
+        return HttpResponse("we were always at war with eastasia")
 
     @route(r'^archive/year/(\d+)/$')
     def archive_by_year(self, request, year):