Improve url and naming consistencies

isic/core/api/collection.py

@@ -17,7 +17,8 @@
 from isic.core.pagination import CursorPagination
 from isic.core.permissions import get_visible_objects
 from isic.core.serializers import SearchQueryIn
-from isic.core.services.collection import collection_create, collection_delete
+from isic.core.services.collection import collection_create
+from isic.core.services.collection import collection_delete as collection_delete_service
 from isic.core.services.collection.image import (
     collection_add_images_from_isic_ids,
     collection_remove_images_from_isic_ids,
@@ -196,15 +197,15 @@ def collection_detail(request, id: int) -> CollectionOut:
     response={204: None, 400: dict, 403: dict},
     include_in_schema=False,
 )
-def collection_delete_(request, id: int):
+def collection_delete(request, id: int):
     qs = get_visible_objects(request.user, "core.view_collection", Collection.objects.all())
     collection = get_object_or_404(qs.distinct(), id=id)
 
     if not request.user.has_perm("core.edit_collection", collection):
         return 403, {"error": "You do not have permission to delete this collection."}
 
     try:
-        collection_delete(collection=collection)
+        collection_delete_service(collection=collection)
     except ValidationError as e:
         return 400, {"error": e.message}
 
@@ -319,7 +320,7 @@ def collection_populate_from_list(request, id, payload: IsicIdList):
 @router.post(
     "/{id}/remove-from-list/", response={200: None, 403: dict, 409: dict}, include_in_schema=False
 )
-def remove_from_list(request, id, payload: IsicIdList):
+def collection_remove_from_list(request, id, payload: IsicIdList):
     qs = get_visible_objects(request.user, "core.view_collection", Collection.objects.all())
     collection = get_object_or_404(qs.distinct(), id=id)
 

isic/core/api/doi.py

@@ -66,7 +66,7 @@ def validate_related_identifiers(cls, v):  # noqa: N805
     summary="Create a draft DOI for a collection.",
     include_in_schema=False,
 )
-def create_doi(request, payload: CreateDOIIn):
+def doi_create(request, payload: CreateDOIIn):
     collection = get_object_or_404(Collection, pk=payload.collection_id)
 
     if not request.user.has_perm("core.create_doi", collection):
@@ -93,7 +93,7 @@ class UpdateDraftDOIIn(Schema):
     summary="Update a draft DOI.",
     include_in_schema=False,
 )
-def update_draft_doi(request, draft_doi_slug: str, payload: UpdateDraftDOIIn):
+def doi_update_draft(request, draft_doi_slug: str, payload: UpdateDraftDOIIn):
     from isic.core.services.collection import collection_update
 
     draft_doi = get_object_or_404(
@@ -116,7 +116,7 @@ def update_draft_doi(request, draft_doi_slug: str, payload: UpdateDraftDOIIn):
     summary="Publish a draft DOI to make it findable.",
     include_in_schema=False,
 )
-def publish_draft_doi(request, draft_doi_slug: str):
+def doi_publish_draft(request, draft_doi_slug: str):
     with transaction.atomic():
         draft_doi = get_object_or_404(
             DraftDoi.objects.select_for_update().select_related("collection"), slug=draft_doi_slug

isic/core/api/image.py

@@ -85,7 +85,7 @@ def resolve_metadata(image: Image) -> dict:
     "/", response=list[ImageOut], summary="Return a list of images.", include_in_schema=True
 )
 @paginate(CursorPagination, ordering=Image._meta.ordering)
-def list_images(request: HttpRequest):
+def image_list(request: HttpRequest):
     qs = get_visible_objects(request.user, "core.view_image", default_qs)
 
     if settings.ISIC_USE_ELASTICSEARCH_COUNTS:
@@ -107,7 +107,7 @@ def list_images(request: HttpRequest):
     include_in_schema=True,
 )
 @paginate(CursorPagination, ordering=Image._meta.ordering)
-def search_images(request: HttpRequest, search: SearchQueryIn = Query(...)):
+def image_search(request: HttpRequest, search: SearchQueryIn = Query(...)):
     try:
         qs = search.to_queryset(user=request.user, qs=default_qs)
         if settings.ISIC_USE_ELASTICSEARCH_COUNTS:
@@ -135,7 +135,7 @@ def search_images(request: HttpRequest, search: SearchQueryIn = Query(...)):
     summary="Get total size of images matching a search query.",
     include_in_schema=False,
 )
-def get_search_size(request: HttpRequest, search: SearchQueryIn = Query(...)):
+def image_search_size(request: HttpRequest, search: SearchQueryIn = Query(...)):
     try:
         es_query = search.to_es_query(request.user)
     except ParseException as e:
@@ -157,8 +157,8 @@ def get_search_size(request: HttpRequest, search: SearchQueryIn = Query(...)):
 
 
 @router.get("/facets/", response=dict, include_in_schema=False)
-def get_facets(request: HttpRequest, search: SearchQueryIn = Query(...)):
-    cache_key = f"get_facets:{search.to_cache_key(request.user)}"
+def image_facets(request: HttpRequest, search: SearchQueryIn = Query(...)):
+    cache_key = f"image_facets:{search.to_cache_key(request.user)}"
     cached_facets = cache.get(cache_key)
 
     set_tag("cached_facets", cached_facets is not None)
@@ -182,7 +182,7 @@ def get_facets(request: HttpRequest, search: SearchQueryIn = Query(...)):
     summary="Retrieve a single image by ISIC ID.",
     include_in_schema=True,
 )
-def retrieve_image(request: HttpRequest, isic_id: str):
+def image_detail(request: HttpRequest, isic_id: str):
     qs = get_visible_objects(request.user, "core.view_image", default_qs)
     return get_object_or_404(qs, isic_id=isic_id)
 
@@ -197,7 +197,7 @@ class SimilarImageOut(ImageOut):
     summary="Find images similar to the specified image.",
     include_in_schema=True,
 )
-def get_similar_images(
+def image_similar(
     request: HttpRequest, isic_id: str, limit: int = Query(10, le=50)
 ) -> list[SimilarImageOut]:
     qs = get_visible_objects(request.user, "core.view_image", default_qs)

isic/core/api/user.py

@@ -42,7 +42,7 @@ def user_me(request: AuthenticatedHttpRequest):
 
 
 @router.put("/accept-terms/", include_in_schema=False, auth=is_authenticated)
-def accept_terms_of_use(request: AuthenticatedHttpRequest):
+def user_accept_terms(request: AuthenticatedHttpRequest):
     if not request.user.profile.accepted_terms:
         request.user.profile.accepted_terms = timezone.now()
         request.user.profile.save(update_fields=["accepted_terms"])

isic/core/templates/core/partials/navbar.html

@@ -8,7 +8,7 @@
         {% include 'core/partials/nav_elements.html' with right_arrow=1 %}
       </ul>
     </div>
-    <a class="btn btn-ghost normal-case text-xl" href="/">ISIC Archive</a>
+    <a class="btn btn-ghost normal-case text-xl" href="{% url 'index' %}">ISIC Archive</a>
   </div>
   <div class="navbar-center hidden lg:flex">
     <ul class="menu menu-horizontal p-0">

isic/core/tests/test_api_cohort.py

@@ -1,3 +1,4 @@
+from django.urls import reverse
 import pytest
 from pytest_lazy_fixtures import lf
 
@@ -28,7 +29,7 @@ def cohorts(cohort, other_cohort):
     ],
 )
 def test_core_api_cohort_list_permissions(client_, cohorts_, num_visible):
-    r = client_.get("/api/v2/cohorts/")
+    r = client_.get(reverse("api:cohort_list"))
 
     assert r.status_code == 200, r.json()
     assert r.json()["count"] == num_visible
@@ -55,7 +56,7 @@ def test_core_api_cohort_list_permissions(client_, cohorts_, num_visible):
     ],
 )
 def test_core_api_cohort_detail_permissions(client_, cohort_, visible):
-    r = client_.get(f"/api/v2/cohorts/{cohort_.pk}/")
+    r = client_.get(reverse("api:cohort_detail", kwargs={"id": cohort_.pk}))
 
     if visible:
         assert r.status_code == 200, r.json()

isic/core/tests/test_api_collection.py

@@ -33,7 +33,7 @@ def test_core_api_collection_autocomplete(
     for name in collection_names:
         collection_factory(name=name, public=True)
 
-    r = client.get(f"/api/v2/collections/autocomplete/?query={search_term}")
+    r = client.get(reverse("api:collection_autocomplete"), {"query": search_term})
     assert r.status_code == 200, r.json()
 
     assert [c["name"] for c in r.json()] == expected_collection_names
@@ -58,7 +58,7 @@ def test_core_api_collection_autocomplete(
     ],
 )
 def test_core_api_collection_list_permissions(client_, colls, num_visible):
-    r = client_.get("/api/v2/collections/")
+    r = client_.get(reverse("api:collection_list"))
 
     assert r.status_code == 200, r.json()
     assert r.json()["count"] == num_visible
@@ -105,7 +105,7 @@ def test_core_api_collection_list_permissions(client_, colls, num_visible):
     ],
 )
 def test_core_api_collection_detail_permissions(client_, collection, visible):
-    r = client_.get(f"/api/v2/collections/{collection.pk}/")
+    r = client_.get(reverse("api:collection_detail", kwargs={"id": collection.pk}))
 
     if visible:
         assert r.status_code == 200, r.json()
@@ -128,7 +128,7 @@ 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/",
+            reverse("api:collection_populate_from_search", kwargs={"id": collection.pk}),
             {"query": "sex:male"},
             content_type="application/json",
         )
@@ -140,18 +140,18 @@ def test_core_api_collection_populate_from_search(
 
 @pytest.mark.django_db
 @pytest.mark.parametrize(
-    ("endpoint", "data"),
+    ("url_name", "data"),
     [
-        ("populate-from-search", {"query": "sex:male"}),
-        ("populate-from-list", {"isic_ids": ["ISIC_0000000"]}),
-        ("remove-from-list", {"isic_ids": ["ISIC_0000000"]}),
+        ("api:collection_populate_from_search", {"query": "sex:male"}),
+        ("api:collection_populate_from_list", {"isic_ids": ["ISIC_0000000"]}),
+        ("api:collection_remove_from_list", {"isic_ids": ["ISIC_0000000"]}),
     ],
 )
-def test_core_api_collection_modify_locked(endpoint, data, staff_client, collection_factory, user):
+def test_core_api_collection_modify_locked(url_name, data, staff_client, collection_factory, user):
     collection = collection_factory(locked=True, creator=user)
 
     r = staff_client.post(
-        f"/api/v2/collections/{collection.pk}/{endpoint}/",
+        reverse(url_name, kwargs={"id": collection.pk}),
         data,
         content_type="application/json",
     )
@@ -173,7 +173,7 @@ def test_core_api_collection_populate_from_list(
     )
 
     r = authenticated_client.post(
-        f"/api/v2/collections/{collection.pk}/populate-from-list/",
+        reverse("api:collection_populate_from_list", kwargs={"id": collection.pk}),
         {
             "isic_ids": [
                 public_image.isic_id,
@@ -210,7 +210,7 @@ def test_core_api_collection_remove_from_list(
     collection.images.add(public_image, private_image_shared, private_image_unshared)
 
     r = authenticated_client.post(
-        f"/api/v2/collections/{collection.pk}/remove-from-list/",
+        reverse("api:collection_remove_from_list", kwargs={"id": collection.pk}),
         {
             "isic_ids": [
                 public_image.isic_id,
@@ -375,7 +375,7 @@ def other_user_client(user_factory):
     ids=["creator", "staff", "other-user", "anonymous"],
 )
 def test_core_api_collection_delete_permissions(client_, deletable_collection, expected_status):
-    r = client_.delete(f"/api/v2/collections/{deletable_collection.pk}/")
+    r = client_.delete(reverse("api:collection_delete", kwargs={"id": deletable_collection.pk}))
 
     assert r.status_code == expected_status
     if expected_status == 204:
@@ -415,7 +415,7 @@ def test_core_api_collection_delete_blocked(
         collection = collection_factory(locked=False, creator=user)
         draft_doi_factory(collection=collection)
 
-    r = authenticated_client.delete(f"/api/v2/collections/{collection.pk}/")
+    r = authenticated_client.delete(reverse("api:collection_delete", kwargs={"id": collection.pk}))
 
     assert r.status_code == 400
     assert Collection.objects.filter(pk=collection.pk).exists()
@@ -439,15 +439,15 @@ def test_core_api_collection_create_from_isic_ids(
     }
 
     r = client.post(
-        "/api/v2/collections/create-from-isic-ids/",
+        reverse("api:collection_create_from_isic_ids"),
         payload,
         content_type="application/json",
     )
     assert r.status_code == 401
 
     with django_capture_on_commit_callbacks(execute=True):
         r = authenticated_client.post(
-            "/api/v2/collections/create-from-isic-ids/",
+            reverse("api:collection_create_from_isic_ids"),
             payload,
             content_type="application/json",
         )

isic/core/tests/test_api_contributor.py

@@ -1,3 +1,4 @@
+from django.urls import reverse
 import pytest
 from pytest_lazy_fixtures import lf
 
@@ -21,7 +22,7 @@
     ],
 )
 def test_core_api_contributor_list_permissions(client_, contributors_, num_visible):
-    r = client_.get("/api/v2/contributors/")
+    r = client_.get(reverse("api:contributor_list"))
 
     assert r.status_code == 200, r.json()
     assert r.json()["count"] == num_visible
@@ -52,7 +53,7 @@ def test_core_api_contributor_list_permissions(client_, contributors_, num_visib
     ],
 )
 def test_core_api_contributor_detail_permissions(client_, contributor_, visible):
-    r = client_.get(f"/api/v2/contributors/{contributor_.pk}/")
+    r = client_.get(reverse("api:contributor_detail", kwargs={"id": contributor_.pk}))
 
     if visible:
         assert r.status_code == 200, r.json()
@@ -64,7 +65,7 @@ def test_core_api_contributor_detail_permissions(client_, contributor_, visible)
 @pytest.mark.django_db
 def test_core_api_contributor_create(authenticated_client, user):
     r = authenticated_client.post(
-        "/api/v2/contributors/",
+        reverse("api:contributor_create"),
         data={
             "institution_name": "string",
             "institution_url": "http://google.com",

isic/core/tests/test_api_image.py

@@ -1,4 +1,5 @@
 from django.db import connection
+from django.urls import reverse
 import pytest
 
 from isic.core.search import add_to_search_index, get_elasticsearch_client
@@ -30,22 +31,22 @@ def test_core_api_image_ages_are_always_rounded(
     authenticated_client, staff_client, searchable_image
 ):
     for client_ in [authenticated_client, staff_client]:
-        r = client_.get("/api/v2/images/")
+        r = client_.get(reverse("api:image_list"))
         assert r.status_code == 200, r.json()
         assert r.json()["count"] == 1
         assert r.json()["results"][0]["metadata"]["clinical"]["age_approx"] == 50
 
-        r = client_.get(f"/api/v2/images/{searchable_image.isic_id}/")
+        r = client_.get(reverse("api:image_detail", kwargs={"isic_id": searchable_image.isic_id}))
         assert r.status_code == 200, r.json()
         assert r.json()["metadata"]["clinical"]["age_approx"] == 50
 
         # test search isn't leaking ages
-        r = client_.get("/api/v2/images/search/", {"query": "age_approx:50"})
+        r = client_.get(reverse("api:image_search"), {"query": "age_approx:50"})
         assert r.status_code == 200, r.json()
         assert r.json()["count"] == 1
         assert r.json()["results"][0]["metadata"]["clinical"]["age_approx"] == 50
 
-        r = client_.get("/api/v2/images/search/", {"query": "age_approx:52"})
+        r = client_.get(reverse("api:image_search"), {"query": "age_approx:52"})
         assert r.status_code == 200, r.json()
         assert r.json()["count"] == 0
 
@@ -55,7 +56,7 @@ def test_core_api_image_ages_are_always_rounded(
 def test_api_image_urls_thumbnail_256(client, image_factory, image_file):
     image = image_factory(public=True)
 
-    api_resp = client.get(f"/api/v2/images/{image.isic_id}/")
+    api_resp = client.get(reverse("api:image_detail", kwargs={"isic_id": image.isic_id}))
 
     assert isinstance(api_resp.json().get("files"), dict)
     assert isinstance(api_resp.json()["files"].get(image_file), dict)
@@ -66,19 +67,19 @@ def test_api_image_urls_thumbnail_256(client, image_factory, image_file):
 def test_api_image_search_size(client, searchable_images_with_size):
     image1, image2 = searchable_images_with_size
 
-    r = client.get("/api/v2/images/search/size/")
+    r = client.get(reverse("api:image_search_size"))
     assert r.status_code == 200
     assert r.json()["size"] == 30  # 10 + 20
 
-    r = client.get("/api/v2/images/search/size/", {"query": "age_approx:10"})
+    r = client.get(reverse("api:image_search_size"), {"query": "age_approx:10"})
     assert r.status_code == 200
     assert r.json()["size"] == 10
 
-    r = client.get("/api/v2/images/search/size/", {"query": "age_approx:20"})
+    r = client.get(reverse("api:image_search_size"), {"query": "age_approx:20"})
     assert r.status_code == 200
     assert r.json()["size"] == 20
 
-    r = client.get("/api/v2/images/search/size/", {"query": "age_approx:30"})
+    r = client.get(reverse("api:image_search_size"), {"query": "age_approx:30"})
     assert r.status_code == 200
     assert r.json()["size"] == 0
 
@@ -97,7 +98,8 @@ def test_api_image_similar_images(client, image_factory, image_embedding_factory
     with connection.cursor() as cursor:
         cursor.execute("SET enable_indexscan = off")
 
-    r = client.get(f"/api/v2/images/{image_with_embedding.isic_id}/similar/")
+    url = reverse("api:image_similar", kwargs={"isic_id": image_with_embedding.isic_id})
+    r = client.get(url)
     assert r.status_code == 200
     results = r.json()
     assert len(results) == 1
@@ -109,6 +111,7 @@ def test_api_image_similar_images(client, image_factory, image_embedding_factory
 def test_api_image_similar_images_without_embedding(client, image_factory):
     image_without_embedding = image_factory(public=True)
 
-    r = client.get(f"/api/v2/images/{image_without_embedding.isic_id}/similar/")
+    url = reverse("api:image_similar", kwargs={"isic_id": image_without_embedding.isic_id})
+    r = client.get(url)
     assert r.status_code == 200
     assert r.json() == []