Ensure new_url is absolute in Redirects

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## Unreleased
 
+### Changed
+- Handling of `new_url` on `RedirectObjectType` to always return absolute URLs ([#391](https://github.com/torchbox/wagtail-grapple/pull/391)) @JakubMastalerz
+
 ## [0.25.1] - 2024-04-21
 
 ### Changed

grapple/settings.py

@@ -72,7 +72,7 @@ def user_settings(self):
 
     def __getattr__(self, attr):
         if attr not in self.defaults:
-            raise AttributeError("Invalid Grapple setting: '%s'" % attr)
+            raise AttributeError(f"Invalid Grapple setting: '{attr}'")
 
         try:
             # Check if present in user settings

grapple/types/redirects.py

@@ -1,6 +1,7 @@
 import copy
 
 from typing import List, Optional
+from urllib.parse import urlparse
 
 import graphene
 
@@ -25,24 +26,39 @@ class RedirectObjectType(graphene.ObjectType):
     class Meta:
         name = "Redirect"
 
-    def resolve_old_url(self, info, **kwargs) -> str:
+    def resolve_old_url(self: Redirect, info, **kwargs) -> str:
         """
         Resolve the value of `old_url` using the `root_url` of the associated
         site and `old_path`.
         """
 
         return self.site.root_url + self.old_path
 
-    def resolve_new_url(self, info, **kwargs) -> Optional[str]:
+    def resolve_new_url(self: Redirect, info, **kwargs) -> Optional[str]:
         """
-        Resolve the value of `new_url`. If `redirect_page` is specified then its
-        URL is prioritised.
+        Resolve the value of `new_url`. If `redirect_page` is specified then
+        `link` is used. Otherwise, ensure that the redirect link is absolute.
         """
+        if self.redirect_page:
+            return self.link  # Handled by the `Redirect` model
+
+        elif self.redirect_link:
+            parsed_url = urlparse(self.redirect_link)
+
+            if not parsed_url.scheme:  # url without scheme is not absolute
+                return (
+                    self.site.root_url.rstrip("/")
+                    + "/"
+                    + self.redirect_link.lstrip("/")
+                )
+            else:
+                return self.redirect_link
 
-        return self.link
+        else:
+            return None
 
     # Return the page that's being redirected to, if at all.
-    def resolve_page(self, info, **kwargs) -> Optional[Page]:
+    def resolve_page(self: Redirect, info, **kwargs) -> Optional[Page]:
         if self.redirect_page is not None:
             return self.redirect_page.specific
 

tests/test_image_types.py

@@ -90,18 +90,15 @@ def test_renditions(self):
     def test_renditions_with_allowed_image_filters_restrictions(self):
         def get_query(**kwargs):
             params = ",".join([f"{key}: {value}" for key, value in kwargs.items()])
-            return (
-                """
-            query ($id: ID!) {
-                image(id: $id) {
-                    rendition(%s) {
-                        url
-                    }
-                }
-            }
-            """
-                % params
-            )
+            return f"""
+                    query ($id: ID!) {{
+                        image(id: $id) {{
+                            rendition({params}) {{
+                                url
+                            }}
+                        }}
+                    }}
+                    """
 
         results = self.client.execute(
             get_query(width=100), variables={"id": self.example_image.id}

tests/test_redirects.py

@@ -277,3 +277,42 @@ def test_query_efficiency(self):
         # There should be one SELECT query for Redirects and one for Sites.
         with self.assertNumQueries(2):
             self.client.execute(query)
+
+    def test_new_url_is_absolute(self):
+        """Test that the `new url` is always an absolute url."""
+
+        # Create a redirect with just `redirect_page`.
+        RedirectFactory(
+            redirect_link="",
+            redirect_page=self.page,
+        )
+        # Create a redirect with absolute url in `redirect_link`.
+        RedirectFactory(
+            redirect_link="http://test.com/",
+            redirect_page=None,
+        )
+
+        # Create a redirect with relative url in `redirect_link`.
+        RedirectFactory(
+            redirect_link="/test",
+            redirect_page=None,
+            site=SiteFactory(
+                hostname="test-site",
+                port=81,
+            ),
+        )
+
+        query = """
+        {
+            redirects {
+                newUrl
+            }
+        }
+        """
+
+        result = self.client.execute(query)["data"]["redirects"]
+
+        # assert all urls are absolute
+        self.assertEqual(result[0]["newUrl"], "http://localhost/test-page-url/")
+        self.assertEqual(result[1]["newUrl"], "http://test.com/")
+        self.assertEqual(result[2]["newUrl"], "http://test-site:81/test")