Redirect image detail URLS to canonical version

Author: danlamanna

Diff for: isic/core/tests/test_image_identifier.py

@@ -0,0 +1,25 @@
+from django.urls import reverse
+import pytest
+
+
+@pytest.mark.django_db
+def test_resolve_isic_id(client, image_factory):
+    image = image_factory(public=True)
+    response = client.get(f"/images/{image.isic_id}/")
+    assert response.status_code == 200
+
+
+@pytest.mark.django_db
+def test_resolve_girder_id(client, image_factory):
+    image = image_factory(public=True, accession__girder_id="5436e3abbae478396759f0cf")
+    response = client.get(f"/images/{image.accession.girder_id}/")
+    assert response.status_code == 301
+    assert response.url == reverse("core/image-detail", kwargs={"image_identifier": image.isic_id})
+
+
+@pytest.mark.django_db
+def test_resolve_pk(client, image_factory):
+    image = image_factory(public=True)
+    response = client.get(f"/images/{image.pk}/")
+    assert response.status_code == 301
+    assert response.url == reverse("core/image-detail", kwargs={"image_identifier": image.isic_id})

Diff for: isic/core/tests/test_view_image_detail.py

@@ -21,7 +21,7 @@
     ],
 )
 def test_core_image_detail(client_, image_, can_see):
-    r = client_.get(reverse("core/image-detail", args=[image_.pk]))
+    r = client_.get(reverse("core/image-detail", args=[image_.isic_id]))
     assert r.status_code == 200 if can_see else 403
 
 
@@ -70,7 +70,7 @@ def detailed_image(
 
 @pytest.mark.django_db
 def test_view_image_detail_public(client, detailed_image):
-    r = client.get(reverse("core/image-detail", args=[detailed_image.pk]))
+    r = client.get(reverse("core/image-detail", args=[detailed_image.isic_id]))
     assert r.status_code == 200
     assert set(r.context["sections"].keys()) == {"metadata", "studies"}
 
@@ -92,7 +92,7 @@ def test_view_image_detail_public(client, detailed_image):
 def test_view_image_detail_uploader(client, detailed_image):
     client.force_login(detailed_image.accession.cohort.contributor.owners.first())
 
-    r = client.get(reverse("core/image-detail", args=[detailed_image.pk]))
+    r = client.get(reverse("core/image-detail", args=[detailed_image.isic_id]))
     assert r.status_code == 200
     assert set(r.context["sections"].keys()) == {"metadata", "studies"}
 

Diff for: isic/core/urls.py

@@ -1,10 +1,6 @@
-from django.db.models import Q
-from django.http.response import Http404
-from django.urls import path, register_converter
+from django.urls import path
 from django.views.generic.base import TemplateView
 
-from isic.core.constants import ISIC_ID_REGEX, MONGO_ID_REGEX
-from isic.core.models.image import Image
 from isic.core.views.collections import (
     collection_create_,
     collection_create_doi_,
@@ -23,33 +19,6 @@
 from isic.core.views.lesion import lesion_detail
 from isic.core.views.users import staff_list, user_detail
 
-
-class ImageIdentifierConverter:
-    regex = f"([0-9]+|{MONGO_ID_REGEX}|{ISIC_ID_REGEX})"
-
-    def to_python(self, value):
-        if value.isnumeric():
-            image = Image.objects.filter(pk=value).first()
-            if image:
-                return image.isic_id
-        else:
-            for approach in [
-                Q(isic_id=value),
-                Q(accession__girder_id=value),
-                Q(aliases__isic=value),
-            ]:
-                image = Image.objects.filter(approach).order_by().first()
-                if image:
-                    return image.isic_id
-
-        raise Http404
-
-    def to_url(self, value):
-        return value
-
-
-register_converter(ImageIdentifierConverter, "image-identifier")
-
 urlpatterns = [
     path(
         "robots.txt",
@@ -92,7 +61,7 @@ def to_url(self, value):
         name="core/collection-download-metadata",
     ),
     path(
-        "images/<image-identifier:isic_id>/",
+        "images/<str:image_identifier>/",
         image_detail,
         name="core/image-detail",
     ),

Diff for: isic/core/views/images.py

@@ -22,6 +22,35 @@
 MAX_RELATED_SHOW_FIRST_N = 50
 
 
+def resolve_image_identifier(view_func):
+    from django.http import HttpResponsePermanentRedirect
+    from django.urls import reverse
+
+    def wrapper(request, image_identifier):
+        if image_identifier.startswith("ISIC_"):
+            return view_func(request, isic_id=image_identifier)
+
+        alternative_identifier_filters = [
+            Q(accession__girder_id=image_identifier),
+        ]
+
+        if image_identifier.isdigit():
+            alternative_identifier_filters.append(Q(pk=image_identifier))
+
+        for approach in alternative_identifier_filters:
+            image = Image.objects.filter(approach).order_by().first()
+            if image:
+                redirect_url = reverse(
+                    "core/image-detail", kwargs={"image_identifier": image.isic_id}
+                )
+                return HttpResponsePermanentRedirect(redirect_url)
+
+        return view_func(request, isic_id=image_identifier)
+
+    return wrapper
+
+
+@resolve_image_identifier
 @needs_object_permission("core.view_image", (Image, "isic_id", "isic_id"))
 def image_detail(request, isic_id):
     image = get_object_or_404(

Diff for: isic/core/views/lesion.py

@@ -48,7 +48,7 @@ def lesion_detail(request, identifier):
             if image.accession.image_type
             else None,
             "isic_id": image.isic_id,
-            "image_detail_url": reverse("core/image-detail", args=[image.id]),
+            "image_detail_url": reverse("core/image-detail", args=[image.isic_id]),
             "acquisition_day": image.accession.acquisition_day,
         }
         for image in images