Remove legacy support for comma separated list params

app/filters/base.py

@@ -9,7 +9,6 @@
 from sqlalchemy.orm import DeclarativeBase
 
 from app.db.model import Identifiable
-from app.logger import L
 
 Aliases = dict[type[Identifiable], type[Identifiable] | dict[str, type[Identifiable]]]
 
@@ -25,29 +24,8 @@ class Constants(Filter.Constants):
 
     @field_validator("*", mode="before")
     @classmethod
-    def split_str(cls, value, field):  # pyright: ignore reportIncompatibleMethodOverride
+    def split_str(cls, value, field):  # pyright: ignore reportIncompatibleMethodOverride  # noqa: ARG003
         """Prevent splitting field logic from parent class."""
-        # backwards compatibility by splitting only comma separated single list elements that do not
-        # have space directly after the comma. e.g "a,b,c" will be split but not 'a, b, c'.
-        if (
-            field.field_name is not None  # noqa: PLR0916
-            and (
-                field.field_name == cls.Constants.ordering_field_name
-                or field.field_name.endswith("__in")
-                or field.field_name.endswith("__not_in")
-            )
-            and value
-            and len(value) == 1
-            and isinstance(value[0], str)
-            and "," in value[0]
-            and ", " not in value[0]
-        ):
-            msg = (
-                "Deprecated comma separated single-string IN query used instead of native list. "
-                f"Filter: field.config['title'] Field name: {field.field_name} Value: {value}"
-            )
-            L.warning(msg)
-            return value[0].split(",")
         return value
 
     @field_validator("order_by", check_fields=False)

app/filters/common.py

@@ -20,11 +20,7 @@
 
 class IdFilterMixin:
     id: uuid.UUID | None = None
-
-    # id__in needs to be a str for backwards compatibility when instead of a native list a comma
-    # separated string is provided, e.g. 'id1,id2' . With list[UUID] backwards compatibility would
-    # fail because of validation of the field which would be expected to be a UUID.
-    id__in: list[str] | None = None
+    id__in: list[uuid.UUID] | None = None
 
 
 class AuthorizedFilterMixin:

tests/test_calibration.py

@@ -232,27 +232,13 @@ def test_filtering(client, models, root_circuit, simulation_result):
     assert len(data) == 2
     """
 
-    # backwards compat
-    data = assert_request(
-        client.get, url=ROUTE, params={"used__id__in": f"{root_circuit.id},{simulation_result.id}"}
-    ).json()["data"]
-    assert len(data) == 5
-
     data = assert_request(
         client.get,
         url=ROUTE,
         params={"used__id__in": [str(root_circuit.id), str(simulation_result.id)]},
     ).json()["data"]
     assert len(data) == 5
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"generated__id__in": f"{root_circuit.id},{simulation_result.id}"},
-    ).json()["data"]
-    assert len(data) == 4
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/test_cell_morphology.py

@@ -649,15 +649,6 @@ def test_filter_by_id__in(db, client, brain_region_id, person_id, subject_id):
 
     # filtering by multiple IDs
     selected_ids = [morphology_ids[1], morphology_ids[3]]
-
-    # backwards compat
-    response = client.get(ROUTE, params={"id__in": ",".join(selected_ids)})
-    assert response.status_code == 200
-    data = response.json()["data"]
-    assert len(data) == 2
-    returned_ids = [item["id"] for item in data]
-    assert set(returned_ids) == set(selected_ids)
-
     response = client.get(ROUTE, params={"id__in": selected_ids})
     assert response.status_code == 200
     data = response.json()["data"]
@@ -666,15 +657,6 @@ def test_filter_by_id__in(db, client, brain_region_id, person_id, subject_id):
     assert set(returned_ids) == set(selected_ids)
 
     # filtering by all IDs
-
-    # backwards compat
-    response = client.get(ROUTE, params={"id__in": ",".join(morphology_ids)})
-    assert response.status_code == 200
-    data = response.json()["data"]
-    assert len(data) == 5
-    returned_ids = [item["id"] for item in data]
-    assert set(returned_ids) == set(morphology_ids)
-
     response = client.get(ROUTE, params={"id__in": morphology_ids})
     assert response.status_code == 200
     data = response.json()["data"]
@@ -689,17 +671,6 @@ def test_filter_by_id__in(db, client, brain_region_id, person_id, subject_id):
     assert len(data) == 0
 
     # combining id__in with other filters
-
-    # backwards compat
-    response = client.get(
-        ROUTE,
-        params={"id__in": ",".join(morphology_ids), "name__ilike": "%Filter Test Morphology 2%"},
-    )
-    assert response.status_code == 200
-    data = response.json()["data"]
-    assert len(data) == 1
-    assert data[0]["id"] == morphology_ids[2]
-
     response = client.get(
         ROUTE,
         params={"id__in": morphology_ids, "name__ilike": "%Filter Test Morphology 2%"},

tests/test_electrical_cell_recording.py

@@ -428,14 +428,6 @@ def test_filtering(client, models):
     ).json()["data"]
     assert {d["name"] for d in data} == {"e-1", "e-2"}
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"name__in": "e-1,e-2"},
-    ).json()["data"]
-    assert {d["name"] for d in data} == {"e-1", "e-2"}
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/test_etype.py

@@ -93,14 +93,6 @@ def test_retrieve(db, client, person_id):
     assert data[0] == with_creation_fields(items[5])
 
     # test filter (in)
-
-    # backwards compat
-    response = client.get(ROUTE, params={"pref_label__in": "pref_label_5,pref_label_6"})
-    assert response.status_code == 200
-    data = response.json()["data"]
-    assert len(data) == 2
-    assert data == [with_creation_fields(items[5]), with_creation_fields(items[6])]
-
     response = client.get(ROUTE, params={"pref_label__in": ["pref_label_5", "pref_label_6"]})
     assert response.status_code == 200
     data = response.json()["data"]

tests/test_experimental_bouton_density.py

@@ -334,14 +334,6 @@ def test_filtering(client, models):
     ).json()["data"]
     assert {d["name"] for d in data} == {"d-1", "d-2"}
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"name__in": "d-1,d-2"},
-    ).json()["data"]
-    assert {d["name"] for d in data} == {"d-1", "d-2"}
-
     data = assert_request(
         client.get, url=ROUTE, params="contribution__pref_label=test_person_1"
     ).json()["data"]

tests/test_experimental_neuron_density.py

@@ -353,14 +353,6 @@ def test_filtering(client, models):
     ).json()["data"]
     assert {d["name"] for d in data} == {"d-1", "d-2"}
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"name__in": "d-1,d-2"},
-    ).json()["data"]
-    assert {d["name"] for d in data} == {"d-1", "d-2"}
-
     data = assert_request(
         client.get, url=ROUTE, params="contribution__pref_label=test_person_1"
     ).json()["data"]

tests/test_ion_channel_recording.py

@@ -443,14 +443,6 @@ def test_filtering(client, models):
     ).json()["data"]
     assert {d["name"] for d in data} == {"e-1", "e-2"}
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"name__in": "e-1,e-2"},
-    ).json()["data"]
-    assert {d["name"] for d in data} == {"e-1", "e-2"}
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/test_mtype.py

@@ -94,13 +94,6 @@ def test_retrieve(db, client, person_id):
     assert data[0] == with_creation_fields(items[5])
 
     # test filter (in)
-    # backwards compat
-    response = client.get(ROUTE, params={"pref_label__in": "pref_label_5,pref_label_6"})
-    assert response.status_code == 200
-    data = response.json()["data"]
-    assert len(data) == 2
-    assert data == [with_creation_fields(items[5]), with_creation_fields(items[6])]
-
     response = client.get(ROUTE, params={"pref_label__in": ["pref_label_5", "pref_label_6"]})
     assert response.status_code == 200
     data = response.json()["data"]

tests/test_ordering.py

@@ -1,5 +1,3 @@
-import pytest
-
 from app.db.model import CellMorphology, License
 
 from tests.utils import PROJECT_ID, add_all_db, check_sort_by_field
@@ -72,6 +70,10 @@ def test_cell_morphology_ordering(db, client, subject_id, license_id, brain_regi
     assert response.status_code == 200
     data = response.json()["data"]
     assert len(data) == count / 2
-    with pytest.raises(AssertionError, match="Items unsorted by id"):
-        check_sort_by_field(data, "id")
     check_sort_by_field(data, "name")
+
+    response = client.get(ROUTE_MORPHOLOGY, params={"name__ilike": "to_find", "order_by": "-name"})
+    assert response.status_code == 200
+    data = response.json()["data"]
+    assert len(data) == count / 2
+    check_sort_by_field(data, "name", how="descending")

tests/test_simulation_execution.py

@@ -223,27 +223,13 @@ def test_filtering(client, models, root_circuit, simulation_result):
     ).json()["data"]
     assert len(data) == 2
 
-    # backwards compat
-    data = assert_request(
-        client.get, url=ROUTE, params={"used__id__in": f"{root_circuit.id},{simulation_result.id}"}
-    ).json()["data"]
-    assert len(data) == 5
-
     data = assert_request(
         client.get,
         url=ROUTE,
         params={"used__id__in": [str(root_circuit.id), str(simulation_result.id)]},
     ).json()["data"]
     assert len(data) == 5
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"generated__id__in": f"{root_circuit.id},{simulation_result.id}"},
-    ).json()["data"]
-    assert len(data) == 4
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/test_simulation_generation.py

@@ -202,27 +202,13 @@ def test_filtering(client, models, root_circuit, simulation_result):
     ).json()["data"]
     assert len(data) == 2
 
-    # backwards compat
-    data = assert_request(
-        client.get, url=ROUTE, params={"used__id__in": f"{root_circuit.id},{simulation_result.id}"}
-    ).json()["data"]
-    assert len(data) == 5
-
     data = assert_request(
         client.get,
         url=ROUTE,
         params={"used__id__in": [str(root_circuit.id), str(simulation_result.id)]},
     ).json()["data"]
     assert len(data) == 5
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"generated__id__in": f"{root_circuit.id},{simulation_result.id}"},
-    ).json()["data"]
-    assert len(data) == 4
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/test_validation.py

@@ -224,27 +224,13 @@ def test_filtering(client, models, root_circuit, simulation_result):
     assert len(data) == 2
     """
 
-    # backwards compat
-    data = assert_request(
-        client.get, url=ROUTE, params={"used__id__in": f"{root_circuit.id},{simulation_result.id}"}
-    ).json()["data"]
-    assert len(data) == 5
-
     data = assert_request(
         client.get,
         url=ROUTE,
         params={"used__id__in": [str(root_circuit.id), str(simulation_result.id)]},
     ).json()["data"]
     assert len(data) == 5
 
-    # backwards compat
-    data = assert_request(
-        client.get,
-        url=ROUTE,
-        params={"generated__id__in": f"{root_circuit.id},{simulation_result.id}"},
-    ).json()["data"]
-    assert len(data) == 4
-
     data = assert_request(
         client.get,
         url=ROUTE,

tests/utils.py

@@ -1,4 +1,5 @@
 import functools
+import operator
 import uuid
 from collections.abc import Callable
 from datetime import UTC, datetime, timedelta
@@ -791,10 +792,18 @@ def delete_entity_classifications(client, client_admin, entity_id):
             )
 
 
-def check_sort_by_field(items, field_name):
-    assert all(items[i][field_name] < items[i + 1][field_name] for i in range(len(items) - 1)), (
-        f"Items unsorted by {field_name}"
-    )
+def check_sort_by_field(items, field_name, how="ascending"):
+    op = {
+        "ascending": operator.le,
+        "descending": operator.ge,
+    }[how]
+    assert all(
+        op(
+            items[i][field_name],
+            items[i + 1][field_name],
+        )
+        for i in range(len(items) - 1)
+    ), f"Items not sorted {how} by {field_name}"
 
 
 def create_subject_ids(db, *, created_by_id, n):