From c5feca261b053824af6fec300e5848c3a93752f0 Mon Sep 17 00:00:00 2001 From: Jakub Mastalerz Date: Tue, 19 Dec 2023 15:47:00 +0100 Subject: [PATCH] Added multi-site handling to RedirectQuery --- CHANGELOG.md | 3 ++- grapple/types/redirects.py | 36 +++++++++++++++++++++++------------- tests/test_redirects.py | 19 ++++++++----------- 3 files changed, 33 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c0a15c7f..52588f93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,12 +4,13 @@ - Factory for `Redirect` model in test app ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz - Test suite for `Redirect` behaviour ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz +- `site` field on `RedirectObjectType` ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz ### Changed - Renamed `RedirectType` to `RedirectObjectType` for clarity ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz - `new_url` field on `RedirectObjectType` is no longer required ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz -- `old_url` field on `RedirectObjectType` now returns url if site is specified and list of urls for all sites if not. ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz +- `RedirectsQuery` now returns a separate redirect object for each existing site when `site=None` ([#380](https://github.com/torchbox/wagtail-grapple/pull/380)) @JakubMastalerz ## [0.23.0] - 2023-09-29 diff --git a/grapple/types/redirects.py b/grapple/types/redirects.py index 567d095f..568a7e1c 100644 --- a/grapple/types/redirects.py +++ b/grapple/types/redirects.py @@ -1,4 +1,4 @@ -import json +import copy from typing import Optional @@ -22,22 +22,15 @@ class RedirectObjectType(graphene.ObjectType): def resolve_old_url(self, info, **kwargs) -> str: """ - If the redirect is for a specific site, append hostname and port to path. - Otherwise, return a JSON string representing a list of all site urls. + Resolve the value of `old_url` using the `root_url` of the associated + site and `old_path`. + Note: `self.site` should never be none because of `resolve_redirects`. """ if self.site: return f"{self.site.root_url}/{self.old_path}" - # TODO: Remove after making site required if self.site is None: - sites_QS = Site.objects.all() - old_url_list = [] - for site in sites_QS: - url = f"http://{site.hostname}:{site.port}/{self.old_path}" - old_url_list.append(url) - - json_string = json.dumps(old_url_list) - return json_string + return None # Get new url def resolve_new_url(self, info, **kwargs) -> Optional[str]: @@ -63,4 +56,21 @@ class RedirectsQuery: # Return all redirects. def resolve_redirects(self, info, **kwargs): - return Redirect.objects.select_related("redirect_page") + """ + Resolve the query set of redirects. If `site` is None, a redirect works + for all sites. To show this, a new redirect object is created for each + of the sites. + """ + redirects_QS = Redirect.objects.select_related("redirect_page") + redirect_list = list(redirects_QS) + sites_QS = Site.objects.all() + for redirect in redirect_list: + if redirect.site is None: + for site in sites_QS: + new_redirect = copy.copy(redirect) + new_redirect.site = site + redirect_list.append(new_redirect) + + redirect_list = filter(lambda x: (x.site is not None), redirect_list) + + return redirect_list diff --git a/tests/test_redirects.py b/tests/test_redirects.py index 0b9eec99..873c0b59 100644 --- a/tests/test_redirects.py +++ b/tests/test_redirects.py @@ -1,5 +1,3 @@ -import json - from test_grapple import BaseGrappleTest from testapp.factories import RedirectFactory from testapp.models import BlogPage @@ -187,11 +185,11 @@ def test_specified_site_url(self): def test_all_sites_url(self): """ - Test that a redirect with no specified site return the desired result - for "all sites". + Test that when no site is specified on a redirect, that redirect is + shown for each existing site. """ - self.site1 = SiteFactory(hostname="test-site", port=8000) - self.site2 = SiteFactory(hostname="another-test-site", port=8001) + self.site1 = SiteFactory(hostname="test-site", port=81) + self.site2 = SiteFactory(hostname="another-test-site", port=82) self.redirect = RedirectFactory( old_path="old-path", @@ -206,9 +204,8 @@ def test_all_sites_url(self): } """ - result = self.client.execute(query)["data"]["redirects"][0]["oldUrl"] - result = json.loads(result) + result = self.client.execute(query)["data"]["redirects"] - self.assertIn("http://localhost:80/old-path", result) - self.assertIn("http://test-site:8000/old-path", result) - self.assertIn("http://another-test-site:8001/old-path", result) + self.assertEqual(result[0]["oldUrl"], "http://another-test-site:82/old-path") + self.assertEqual(result[1]["oldUrl"], "http://localhost/old-path") + self.assertEqual(result[2]["oldUrl"], "http://test-site:81/old-path")