Repository: wagtail/wagtail
Pull request: Retain fields parameter when redirecting from find_view of API
Fixes #6577

I know there must be prettier ways to accomplish this but I'm not entirely familiar with what tools there are in Wagtail to construct URLs so here's a starting point.

Problem statement:
Keep queries other than html_path in /api/v2/pages/find/
### Is your proposal related to a problem?

In wagtail API, I often just want the specific field of the found page by `/pages/find/`. but `/pages/find/?html_path=/&fields=_,id` will unfortunately redirect to `/pages/:id/` and fields query disappeared.

### Describe the solution you'd like

Keep queries other than html_path in /api/v2/pages/find/

### Describe alternatives you've considered

I am using the ajax to fetch from the API so I cannot simply manually custom the redirected URL, but using a proxy with some server logic would workaround this.

### Additional context

I could try providing a PR if this proposal would be approved.

Hints from issue discussion:

Commit message: Retain fields parameter when redirecting from find_view of API

Patch to review:
diff --git a/wagtail/api/v2/tests/test_pages.py b/wagtail/api/v2/tests/test_pages.py
index 06a4e1bcdeb6..5a803a72bcd3 100644
--- a/wagtail/api/v2/tests/test_pages.py
+++ b/wagtail/api/v2/tests/test_pages.py
@@ -1781,6 +1781,19 @@ def test_find_by_html_path_with_start_and_end_slashes_removed(self):
             fetch_redirect_response=False,
         )
 
+    def test_find_by_html_path_and_define_fields(self):
+        response = self.get_response(
+            html_path="/events-index/event-1/", fields="_,id,type"
+        )
+
+        self.assertRedirects(
+            response,
+            "http://localhost"
+            + reverse("wagtailapi_v2:pages:detail", args=[8])
+            + "?fields=_,id,type",
+            fetch_redirect_response=False,
+        )
+
     def test_find_by_html_path_nonexistent(self):
         response = self.get_response(html_path="/foo")
 
diff --git a/wagtail/api/v2/views.py b/wagtail/api/v2/views.py
index fb08642a4d94..8b2d22ae8d1b 100644
--- a/wagtail/api/v2/views.py
+++ b/wagtail/api/v2/views.py
@@ -113,6 +113,9 @@ def find_view(self, request):
                 )
             )
 
+        if "fields" in request.GET:
+            url = url + "?fields=" + request.GET["fields"]
+
         return redirect(url)
 
     def find_object(self, queryset, request):


Code review comments:
diff_hunk: @@ -113,6 +113,9 @@ def find_view(self, request):
                 )
             )
 
+        if "fields" in request.GET:
+            url = url + "?fields=" + request.GET["fields"]
path: wagtail/api/v2/views.py
text: @user1:
I'm not sure which one is better:
1. retain all query parameters in `request.GET` except ones used for finding the object i.e. `html_path` in this case, or
2. only retain `fields` explicitly and disregard all other parameters

In practice it doesn't really matter because I believe `fields` is the only query parameter supported by the detail view at the moment. If we add more in the future, then we'll need to make sure the same is applied here.

Although looking at the code, it seems this view is in the generic `BaseAPIViewSet`, while `html_path` is specific to `PageAPIViewSet`. This means if we want to do the (1) approach, we need to have something like `find_query_parameters = []` in the `BaseAPIViewSet` that is then overridden to be `["html_path"]` in the `PageAPIViewSet`. Then, do a `querydict.pop(q, None)` for each element in the list. 

The issue in #6577 seems to lean towards (1), and I tend to agree. I've got it working locally so I'll push the changes and merge. Thanks!