From b661179eb21e965a683f800c1205c9849da6403c Mon Sep 17 00:00:00 2001 From: Brian Helba Date: Wed, 27 May 2026 18:17:55 -0400 Subject: [PATCH] Use `reverse(query=)` to build URLs with query strings This also removed the recently-added `_build_url` test helper, as it's obsoleted by this. --- isic/core/tests/test_collection_list_browser.py | 14 ++++++-------- isic/core/tests/test_data_explorer_browser.py | 3 +-- isic/core/tests/test_permissions.py | 4 ++-- 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/isic/core/tests/test_collection_list_browser.py b/isic/core/tests/test_collection_list_browser.py index e267034fa..07c63bb8e 100644 --- a/isic/core/tests/test_collection_list_browser.py +++ b/isic/core/tests/test_collection_list_browser.py @@ -13,12 +13,6 @@ def _refresh_collection_counts(): cursor.execute("REFRESH MATERIALIZED VIEW materialized_collection_counts;") -def _build_url(**params): - base = reverse("core/collection-list") - qs = "&".join(f"{k}={v}" for k, v in params.items()) - return f"{base}?{qs}" - - @pytest.mark.playwright def test_collection_list_desktop( staff_authenticated_page, @@ -58,7 +52,9 @@ def test_collection_list_desktop( related_identifier="https://example.com/paper", ) - page.goto(_build_url(exclude_empty=0, magic_filter="exclude")) + page.goto( + reverse("core/collection-list", query={"exclude_empty": 0, "magic_filter": "exclude"}) + ) # All four column headers are visible on desktop expect(page.get_by_role("columnheader", name="Created")).to_be_visible() @@ -138,7 +134,9 @@ def test_collection_list_mobile( ctx.add_cookies([{"name": "sessionid", "value": session_cookie.value, "url": live_server.url}]) page = ctx.new_page() - page.goto(_build_url(exclude_empty=0, magic_filter="exclude")) + page.goto( + reverse("core/collection-list", query={"exclude_empty": 0, "magic_filter": "exclude"}) + ) # Name column is visible on mobile expect(page.get_by_role("columnheader", name="Name")).to_be_visible() diff --git a/isic/core/tests/test_data_explorer_browser.py b/isic/core/tests/test_data_explorer_browser.py index 089a73b28..c2a586506 100644 --- a/isic/core/tests/test_data_explorer_browser.py +++ b/isic/core/tests/test_data_explorer_browser.py @@ -1,7 +1,6 @@ from datetime import UTC, datetime from pathlib import Path import tempfile -from urllib.parse import quote from django.core.files.storage import storages from django.urls import reverse @@ -125,7 +124,7 @@ def test_data_explorer_example_query(page, data_explorer_parquet): def test_data_explorer_query_sharing_via_link(page, data_explorer_parquet): query = "SELECT sex, COUNT(*) AS count FROM metadata GROUP BY sex ORDER BY count DESC" page.goto( - f"{reverse('core/data-explorer')}?q={quote(query)}", + reverse("core/data-explorer", query={"q": query}), timeout=30_000, ) _wait_for_ready(page) diff --git a/isic/core/tests/test_permissions.py b/isic/core/tests/test_permissions.py index 20141a76f..861c9aef6 100644 --- a/isic/core/tests/test_permissions.py +++ b/isic/core/tests/test_permissions.py @@ -58,7 +58,7 @@ def test_core_collection_create(client_, visible): @pytest.mark.django_db def test_core_collection_list(client, authenticated_client, staff_client, private_collection): - url = reverse("core/collection-list") + "?exclude_empty=0&magic_filter=all" + url = reverse("core/collection-list", query={"exclude_empty": "0", "magic_filter": "all"}) r = client.get(url) assert list(r.context["page"].object_list) == [] @@ -86,7 +86,7 @@ def test_core_collection_detail(client, authenticated_client, staff_client, priv def test_core_collection_list_shares( user, client, authenticated_client, staff_client, private_collection ): - url = reverse("core/collection-list") + "?exclude_empty=0&magic_filter=all" + url = reverse("core/collection-list", query={"exclude_empty": "0", "magic_filter": "all"}) private_collection.shares.add(user, through_defaults={"grantor": private_collection.creator}) r = client.get(url)