#2281 fix issue when multiple redirects are returned for a path

wagtail/wagtailredirects/middleware.py

@@ -27,6 +27,9 @@ class RedirectMiddleware(object):
         # Find redirect
         try:
             redirect = models.Redirect.get_for_site(request.site).get(old_path=path)
+        except models.Redirect.MultipleObjectsReturned:
+            # We have a site-specific and a site-ambivalent redirect; prefer the specific one
+            redirect = models.Redirect.objects.get(site=request.site, old_path=path)
         except models.Redirect.DoesNotExist:
             if path == path_without_query:
                 # don't try again if we know we will get the same response

wagtail/wagtailredirects/tests.py

@@ -159,6 +159,46 @@ class TestRedirects(TestCase):
         response = self.client.get('/xmas/', HTTP_HOST='localhost')
         self.assertEqual(response.status_code, 404)
 
+    def test_duplicate_redirects_when_match_is_for_generic(self):
+        contact_page = Page.objects.get(url_path='/home/contact-us/')
+        site = Site.objects.create(hostname='other.example.com', port=80, root_page=contact_page)
+        christmas_page = Page.objects.get(url_path='/home/events/christmas/')
+
+        # two redirects, one for any site, one for specific
+        models.Redirect.objects.create(old_path='/xmas', redirect_page=contact_page)
+        models.Redirect.objects.create(site=site, old_path='/xmas', redirect_page=christmas_page)
+
+        response = self.client.get('/xmas/', HTTP_HOST='localhost')
+        # the redirect which matched was the generic one, pointing to contact_page
+        self.assertRedirects(response, 'http://other.example.com/', status_code=301, fetch_redirect_response=False)
+
+    def test_duplicate_redirects_across_sites_when_match_is_for_specific(self):
+        contact_page = Page.objects.get(url_path='/home/contact-us/')
+        site = Site.objects.create(hostname='other.example.com', port=80, root_page=contact_page)
+        christmas_page = Page.objects.get(url_path='/home/events/christmas/')
+
+        # two redirects, one for any site, one for specific
+        models.Redirect.objects.create(old_path='/xmas', redirect_page=contact_page)
+        models.Redirect.objects.create(site=site, old_path='/xmas', redirect_page=christmas_page)
+
+        # request for specific site gets the christmas_page redirect, not accessible from other.example.com
+        response = self.client.get('/xmas/', HTTP_HOST='other.example.com')
+        self.assertRedirects(response, 'http://localhost/events/christmas/', status_code=301, fetch_redirect_response=False)
+
+    def test_duplicate_redirects_within_a_site_when_match_is_for_specific(self):
+        businessy_page = Page.objects.get(url_path='/home/events/businessy-events/')
+        site = Site.objects.create(hostname='other.example.com', port=80, root_page=businessy_page)
+        board_meetings_page = Page.objects.get(url_path='/home/events/businessy-events/board-meetings/')
+        christmas_page = Page.objects.get(url_path='/home/events/christmas/')
+
+        # two redirects, one for any site, one for specific
+        models.Redirect.objects.create(old_path='/xmas', redirect_page=christmas_page)
+        models.Redirect.objects.create(site=site, old_path='/xmas', redirect_page=board_meetings_page)
+
+        # request for specific site gets the meetings_page redirect, which is accessible from that site
+        response = self.client.get('/xmas/', HTTP_HOST='other.example.com')
+        self.assertRedirects(response, 'http://other.example.com/board-meetings/', status_code=301, fetch_redirect_response=False)
+
 
 class TestRedirectsIndexView(TestCase, WagtailTestUtils):
     def setUp(self):