Update tests to use Django's test client

isic/conftest.py

@@ -44,6 +44,11 @@ def api_client() -> APIClient:
     return APIClient()
 
 
+@pytest.fixture
+def client() -> Client:
+    return Client()
+
+
 @pytest.fixture
 def authenticated_client(user):
     # Do not use the client fixture, to prevent mutating its state

isic/core/api/collection.py

@@ -42,7 +42,7 @@ def collection_list(request, pinned: bool | None = None) -> list[CollectionOut]:
     return queryset
 
 
-@router.get("/{id}", response=CollectionOut)
+@router.get("/{id}/", response=CollectionOut)
 def collection_detail(request, id) -> CollectionOut:
     qs = get_visible_objects(request.user, "core.view_collection", Collection.objects.all())
     collection = get_object_or_404(qs, id=id)
@@ -51,6 +51,7 @@ def collection_detail(request, id) -> CollectionOut:
 
 @router.post("/{id}/populate-from-search/", response={202: None, 403: dict, 409: dict})
 def collection_populate_from_search(request, id, payload: SearchQueryIn):
+    breakpoint()
     collection = get_object_or_404(Collection, id=id)
 
     if not request.user.has_perm("core.add_images", collection):

isic/core/serializers.py

@@ -7,7 +7,7 @@
 from django.shortcuts import get_object_or_404
 from isic_metadata import FIELD_REGISTRY
 from ninja import Schema
-from pydantic import validator
+from pydantic import validator, field_validator
 from pyparsing.exceptions import ParseException
 from rest_framework import serializers
 from rest_framework.fields import Field
@@ -89,7 +89,7 @@ def to_queryset(self, qs: Optional[QuerySet[Image]] = None) -> QuerySet[Image]:
 # Update this to use context for the user once django-ninja supports it
 class SearchQueryIn(Schema):
     query: str | None
-    collections: list[int] | None
+    collections: list[int] | None = None
 
     @validator("query")
     @classmethod
@@ -100,6 +100,7 @@ def valid_search_query(cls, value: str):
         try:
             parse_query(value)
         except ParseException:
+            # TODO this doesn't do the right thing in ninja...
             raise ValueError("Couldn't parse search query.")
         return value
 

isic/core/tests/test_api_collection.py

@@ -8,47 +8,47 @@ def collections(public_collection, private_collection):
 
 @pytest.mark.django_db
 @pytest.mark.parametrize(
-    "client,colls,num_visible",
+    "client_,colls,num_visible",
     [
-        [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("collections"), 1],
+        [pytest.lazy_fixture("client"), pytest.lazy_fixture("collections"), 1],
         [
-            pytest.lazy_fixture("authenticated_api_client"),
+            pytest.lazy_fixture("authenticated_client"),
             pytest.lazy_fixture("collections"),
             1,
         ],
-        [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("collections"), 2],
+        [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("collections"), 2],
     ],
     ids=[
         "guest",
         "user",
         "staff",
     ],
 )
-def test_core_api_collection_list_permissions(client, colls, num_visible):
-    r = client.get("/api/v2/collections/")
+def test_core_api_collection_list_permissions(client_, colls, num_visible):
+    r = client_.get("/api/v2/collections/")
 
     assert r.status_code == 200, r.json()
     assert r.json()["count"] == num_visible
 
 
 @pytest.mark.django_db
 @pytest.mark.parametrize(
-    "client,collection,visible",
+    "client_,collection,visible",
     [
-        [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("public_collection"), True],
+        [pytest.lazy_fixture("client"), pytest.lazy_fixture("public_collection"), True],
         [
-            pytest.lazy_fixture("authenticated_api_client"),
+            pytest.lazy_fixture("authenticated_client"),
             pytest.lazy_fixture("public_collection"),
             True,
         ],
-        [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("public_collection"), True],
-        [pytest.lazy_fixture("api_client"), pytest.lazy_fixture("private_collection"), False],
+        [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("public_collection"), True],
+        [pytest.lazy_fixture("client"), pytest.lazy_fixture("private_collection"), False],
         [
-            pytest.lazy_fixture("authenticated_api_client"),
+            pytest.lazy_fixture("authenticated_client"),
             pytest.lazy_fixture("private_collection"),
             False,
         ],
-        [pytest.lazy_fixture("staff_api_client"), pytest.lazy_fixture("private_collection"), True],
+        [pytest.lazy_fixture("staff_client"), pytest.lazy_fixture("private_collection"), True],
     ],
     ids=[
         "guest-public",
@@ -59,8 +59,8 @@ def test_core_api_collection_list_permissions(client, colls, num_visible):
         "staff-private",
     ],
 )
-def test_core_api_collection_detail_permissions(client, collection, visible):
-    r = client.get(f"/api/v2/collections/{collection.pk}/")
+def test_core_api_collection_detail_permissions(client_, collection, visible):
+    r = client_.get(f"/api/v2/collections/{collection.pk}/")
 
     if visible:
         assert r.status_code == 200, r.json()
@@ -84,7 +84,8 @@ def test_core_api_collection_populate_from_search(
 
     with django_capture_on_commit_callbacks(execute=True):
         r = authenticated_client.post(
-            f"/api/v2/collections/{collection.pk}/populate-from-search/", {"query": "sex:male"}
+            f"/api/v2/collections/{collection.pk}/populate-from-search/", {"query": "sex:male"},
+            content_type='application/json'
         )
 
     assert r.status_code == 202, r.json()
@@ -111,7 +112,7 @@ def test_core_api_collection_modify_locked(endpoint, data, staff_client, collect
 
 @pytest.mark.django_db
 def test_core_api_collection_populate_from_list(
-    authenticated_api_client, collection_factory, image_factory, user
+    authenticated_client, collection_factory, image_factory, user
 ):
     collection = collection_factory(locked=False, creator=user, public=True)
     public_image = image_factory(accession__metadata={"sex": "male"}, public=True)
@@ -122,7 +123,7 @@ def test_core_api_collection_populate_from_list(
         user, through_defaults={"creator": private_image_shared.accession.creator}
     )
 
-    r = authenticated_api_client.post(
+    r = authenticated_client.post(
         f"/api/v2/collections/{collection.pk}/populate-from-list/",
         {
             "isic_ids": [
@@ -145,7 +146,7 @@ def test_core_api_collection_populate_from_list(
 
 @pytest.mark.django_db
 def test_core_api_collection_remove_from_list(
-    authenticated_api_client, collection_factory, image_factory, user
+    authenticated_client, collection_factory, image_factory, user
 ):
     collection = collection_factory(locked=False, creator=user, public=False)
     public_image = image_factory(accession__metadata={"sex": "male"}, public=True)
@@ -158,7 +159,7 @@ def test_core_api_collection_remove_from_list(
 
     collection.images.add(public_image, private_image_shared, private_image_unshared)
 
-    r = authenticated_api_client.post(
+    r = authenticated_client.post(
         f"/api/v2/collections/{collection.pk}/remove-from-list/",
         {
             "isic_ids": [