diff --git a/isic/core/admin.py b/isic/core/admin.py index d5446337..adc0d47a 100644 --- a/isic/core/admin.py +++ b/isic/core/admin.py @@ -4,7 +4,15 @@ from django.utils.html import format_html from resonant_utils.admin import ReadonlyTabularInline -from isic.core.models import Collection, Doi, GirderDataset, GirderImage, Image, ImageAlias +from isic.core.models import ( + Collection, + Doi, + GirderDataset, + GirderImage, + Image, + ImageAlias, + SimilarImageFeedback, +) from isic.core.models.doi import DoiRelatedIdentifier, DraftDoi, DraftDoiRelatedIdentifier from isic.core.models.segmentation import Segmentation, SegmentationReview from isic.core.models.supplemental_file import DraftSupplementalFile, SupplementalFile @@ -261,3 +269,30 @@ class DraftDoiAdmin(BaseDoiAdmin): "is_publishing", ] readonly_fields = ["is_publishing"] + + +@admin.register(SimilarImageFeedback) +class SimilarImageFeedbackAdmin(StaffReadonlyAdmin): + list_display = ["id", "created", "user", "image_link", "similar_image_link", "feedback"] + list_filter = ["feedback", "created"] + search_fields = ["user__username", "image__isic_id", "similar_image__isic_id"] + date_hierarchy = "created" + ordering = ["-created"] + + def image_link(self, obj): + return format_html( + '{}', + obj.image.isic_id, + obj.image.isic_id, + ) + + image_link.short_description = "Source Image" + + def similar_image_link(self, obj): + return format_html( + '{}', + obj.similar_image.isic_id, + obj.similar_image.isic_id, + ) + + similar_image_link.short_description = "Similar Image" diff --git a/isic/core/api/image.py b/isic/core/api/image.py index f5855d9a..092477a3 100644 --- a/isic/core/api/image.py +++ b/isic/core/api/image.py @@ -11,7 +11,7 @@ from pyparsing.exceptions import ParseException from sentry_sdk import set_tag -from isic.core.models import Image +from isic.core.models import Image, SimilarImageFeedback from isic.core.pagination import CursorPagination, qs_with_hardcoded_count from isic.core.permissions import get_visible_objects from isic.core.search import facets, get_elasticsearch_client @@ -189,3 +189,44 @@ def get_facets(request: HttpRequest, search: SearchQueryIn = Query(...)): def retrieve_image(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) + + +class SimilarImageFeedbackIn(Schema): + similar_image_id: str + feedback: str + + +@router.post( + "/{isic_id}/similar-feedback/", + response={200: dict, 400: dict, 401: dict}, + summary="Submit feedback for a similar image recommendation.", + include_in_schema=True, +) +def submit_similar_image_feedback(request: HttpRequest, isic_id: str, data: SimilarImageFeedbackIn): + if not request.user.is_authenticated: + return 401, {"message": "Authentication required"} + + # Validate feedback value + if data.feedback not in [SimilarImageFeedback.THUMBS_UP, SimilarImageFeedback.THUMBS_DOWN]: + return 400, {"message": "Invalid feedback value"} + + # Get the source image + qs = get_visible_objects(request.user, "core.view_image", default_qs) + source_image = get_object_or_404(qs, isic_id=isic_id) + + # Get the similar image + similar_image = get_object_or_404(qs, isic_id=data.similar_image_id) + + # Create or update feedback + feedback, created = SimilarImageFeedback.objects.update_or_create( + image=source_image, + similar_image=similar_image, + user=request.user, + defaults={"feedback": data.feedback}, + ) + + return 200, { + "message": "Feedback submitted successfully", + "created": created, + "feedback": feedback.feedback, + } diff --git a/isic/core/migrations/0036_similarimagefeedback.py b/isic/core/migrations/0036_similarimagefeedback.py new file mode 100644 index 00000000..f7d19ab4 --- /dev/null +++ b/isic/core/migrations/0036_similarimagefeedback.py @@ -0,0 +1,81 @@ +# Generated manually for similar image feedback feature + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django_extensions.db.fields + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0035_image_image_embedding_ivfflat_idx"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="SimilarImageFeedback", + fields=[ + ( + "id", + models.BigAutoField( + auto_created=True, primary_key=True, serialize=False, verbose_name="ID" + ), + ), + ( + "created", + django_extensions.db.fields.CreationDateTimeField( + auto_now_add=True, verbose_name="created" + ), + ), + ( + "modified", + django_extensions.db.fields.ModificationDateTimeField( + auto_now=True, verbose_name="modified" + ), + ), + ( + "feedback", + models.CharField( + choices=[("up", "Thumbs Up"), ("down", "Thumbs Down")], max_length=10 + ), + ), + ( + "image", + models.ForeignKey( + help_text="The source image being viewed", + on_delete=django.db.models.deletion.CASCADE, + related_name="similarity_feedback_source", + to="core.image", + ), + ), + ( + "similar_image", + models.ForeignKey( + help_text="The similar image being rated", + on_delete=django.db.models.deletion.CASCADE, + related_name="similarity_feedback_target", + to="core.image", + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL + ), + ), + ], + options={ + "verbose_name": "Similar Image Feedback", + "verbose_name_plural": "Similar Image Feedback", + "get_latest_by": "modified", + "abstract": False, + }, + ), + migrations.AddConstraint( + model_name="similarimagefeedback", + constraint=models.UniqueConstraint( + fields=("image", "similar_image", "user"), name="similar_image_feedback_unique" + ), + ), + ] diff --git a/isic/core/models/__init__.py b/isic/core/models/__init__.py index d87e7da7..a8edff16 100644 --- a/isic/core/models/__init__.py +++ b/isic/core/models/__init__.py @@ -7,7 +7,7 @@ from .collection_count import CollectionCount from .doi import Doi from .girder_image import GirderDataset, GirderImage -from .image import Image +from .image import Image, SimilarImageFeedback from .image_alias import ImageAlias from .isic_id import IsicId from .segmentation import Segmentation, SegmentationReview @@ -27,6 +27,7 @@ "IsicOAuthApplication", "Segmentation", "SegmentationReview", + "SimilarImageFeedback", "SupplementalFile", ] diff --git a/isic/core/models/image.py b/isic/core/models/image.py index a06b312c..e8d23f02 100644 --- a/isic/core/models/image.py +++ b/isic/core/models/image.py @@ -260,6 +260,53 @@ class Meta(TimeStampedModel.Meta): grantee = models.ForeignKey(User, on_delete=models.CASCADE) +class SimilarImageFeedback(TimeStampedModel): + """ + Feedback for similar image search results. + + Allows authenticated users to provide thumbs up/down feedback on similar image + recommendations for auditing purposes. + """ + + class Meta(TimeStampedModel.Meta): + constraints = [ + models.UniqueConstraint( + name="similar_image_feedback_unique", + fields=["image", "similar_image", "user"], + ), + ] + verbose_name = "Similar Image Feedback" + verbose_name_plural = "Similar Image Feedback" + + THUMBS_UP = "up" + THUMBS_DOWN = "down" + FEEDBACK_CHOICES = [ + (THUMBS_UP, "Thumbs Up"), + (THUMBS_DOWN, "Thumbs Down"), + ] + + image = models.ForeignKey( + Image, + on_delete=models.CASCADE, + related_name="similarity_feedback_source", + help_text="The source image being viewed", + ) + similar_image = models.ForeignKey( + Image, + on_delete=models.CASCADE, + related_name="similarity_feedback_target", + help_text="The similar image being rated", + ) + user = models.ForeignKey(User, on_delete=models.CASCADE) + feedback = models.CharField(max_length=10, choices=FEEDBACK_CHOICES) + + def __str__(self): + return ( + f"{self.user.username}: {self.image.isic_id} -> " + f"{self.similar_image.isic_id} ({self.feedback})" + ) + + class ImagePermissions: model = Image perms = ["view_image", "view_full_metadata"] diff --git a/isic/core/templates/core/image_detail/base.html b/isic/core/templates/core/image_detail/base.html index 62544302..583ca81c 100644 --- a/isic/core/templates/core/image_detail/base.html +++ b/isic/core/templates/core/image_detail/base.html @@ -75,7 +75,7 @@ {% include 'core/image_detail/studies_tab.html' %} {% if 'similar_images' in sections %} - {% include 'core/image_detail/images_tab.html' with images=similar_images section_name='similar_images' lazy=1 %} + {% include 'core/image_detail/similar_images_tab.html' with source_image=image similar_images_count=similar_images.count %} {% endif %} {% if 'patient_images' in sections %} diff --git a/isic/core/templates/core/image_detail/similar_images_tab.html b/isic/core/templates/core/image_detail/similar_images_tab.html new file mode 100644 index 00000000..1b6e7653 --- /dev/null +++ b/isic/core/templates/core/image_detail/similar_images_tab.html @@ -0,0 +1,55 @@ +
+
+
+ +
+ {% for image in similar_images|slice:MAX_RELATED_SHOW_FIRST_N %} + {% include 'core/partials/similar_image_with_feedback.html' with source_image_id=source_image.isic_id %} + {% endfor %} +
+ + {% if similar_images_count and similar_images_count > MAX_RELATED_SHOW_FIRST_N %} +
Showing first {{ MAX_RELATED_SHOW_FIRST_N }} images.
+ {% endif %} +
+ + {% include 'ingest/partials/thumbnail_grid_js.html' %} +
+
+ + diff --git a/isic/core/templates/core/partials/similar_image_with_feedback.html b/isic/core/templates/core/partials/similar_image_with_feedback.html new file mode 100644 index 00000000..0d971377 --- /dev/null +++ b/isic/core/templates/core/partials/similar_image_with_feedback.html @@ -0,0 +1,38 @@ +{% load accession %} + +
+
+ {% block thumb %} + {% include 'core/partials/image_thumb.html' %} + {% endblock %} +
+ + {% block below_thumb %} +
+
+ {{image.isic_id}} + + + + + + +
+ + {% if user.is_authenticated %} +
+ + +
+ {% endif %} +
+ {% endblock %} +
diff --git a/isic/core/tests/test_similar_image_feedback.py b/isic/core/tests/test_similar_image_feedback.py new file mode 100644 index 00000000..66e71cfd --- /dev/null +++ b/isic/core/tests/test_similar_image_feedback.py @@ -0,0 +1,216 @@ +from django.db import IntegrityError +import pytest + +from isic.core.models import SimilarImageFeedback + + +@pytest.mark.django_db +def test_similar_image_feedback_unauthenticated(client, image_factory): + """Test that unauthenticated users cannot submit feedback.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + response = client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "up", + }, + content_type="application/json", + ) + + assert response.status_code == 401 + assert response.json()["message"] == "Authentication required" + assert SimilarImageFeedback.objects.count() == 0 + + +@pytest.mark.django_db +def test_similar_image_feedback_thumbs_up(authenticated_client, user, image_factory): + """Test that authenticated users can submit thumbs up feedback.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "up", + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["message"] == "Feedback submitted successfully" + assert response.json()["created"] is True + assert response.json()["feedback"] == "up" + + # Verify feedback was saved + feedback = SimilarImageFeedback.objects.get() + assert feedback.image == source_image + assert feedback.similar_image == similar_image + assert feedback.user == user + assert feedback.feedback == SimilarImageFeedback.THUMBS_UP + + +@pytest.mark.django_db +def test_similar_image_feedback_thumbs_down(authenticated_client, user, image_factory): + """Test that authenticated users can submit thumbs down feedback.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "down", + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["message"] == "Feedback submitted successfully" + assert response.json()["created"] is True + assert response.json()["feedback"] == "down" + + # Verify feedback was saved + feedback = SimilarImageFeedback.objects.get() + assert feedback.image == source_image + assert feedback.similar_image == similar_image + assert feedback.user == user + assert feedback.feedback == SimilarImageFeedback.THUMBS_DOWN + + +@pytest.mark.django_db +def test_similar_image_feedback_update(authenticated_client, user, image_factory): + """Test that submitting feedback again updates the existing record.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + # Submit initial thumbs up + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "up", + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["created"] is True + assert SimilarImageFeedback.objects.count() == 1 + + # Update to thumbs down + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "down", + }, + content_type="application/json", + ) + + assert response.status_code == 200 + assert response.json()["created"] is False + assert response.json()["feedback"] == "down" + + # Verify only one feedback record exists and it's updated + assert SimilarImageFeedback.objects.count() == 1 + feedback = SimilarImageFeedback.objects.get() + assert feedback.feedback == SimilarImageFeedback.THUMBS_DOWN + + +@pytest.mark.django_db +def test_similar_image_feedback_invalid_value(authenticated_client, image_factory): + """Test that invalid feedback values are rejected.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": similar_image.isic_id, + "feedback": "invalid", + }, + content_type="application/json", + ) + + assert response.status_code == 400 + assert response.json()["message"] == "Invalid feedback value" + assert SimilarImageFeedback.objects.count() == 0 + + +@pytest.mark.django_db +def test_similar_image_feedback_nonexistent_image(authenticated_client, image_factory): + """Test that feedback for nonexistent images returns 404.""" + source_image = image_factory(public=True) + + response = authenticated_client.post( + f"/api/v2/images/{source_image.isic_id}/similar-feedback/", + data={ + "similar_image_id": "ISIC_9999999", + "feedback": "up", + }, + content_type="application/json", + ) + + assert response.status_code == 404 + assert SimilarImageFeedback.objects.count() == 0 + + +@pytest.mark.django_db +def test_similar_image_feedback_source_image_not_found(authenticated_client): + """Test that feedback for nonexistent source image returns 404.""" + response = authenticated_client.post( + "/api/v2/images/ISIC_9999999/similar-feedback/", + data={ + "similar_image_id": "ISIC_9999998", + "feedback": "up", + }, + content_type="application/json", + ) + + assert response.status_code == 404 + assert SimilarImageFeedback.objects.count() == 0 + + +@pytest.mark.django_db +def test_similar_image_feedback_unique_constraint(user, image_factory): + """Test that the unique constraint on image, similar_image, and user works.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + # Create first feedback + SimilarImageFeedback.objects.create( + image=source_image, + similar_image=similar_image, + user=user, + feedback=SimilarImageFeedback.THUMBS_UP, + ) + + # Trying to create another feedback with same combination should work + # because update_or_create handles it, but direct create should fail + with pytest.raises(IntegrityError): + SimilarImageFeedback.objects.create( + image=source_image, + similar_image=similar_image, + user=user, + feedback=SimilarImageFeedback.THUMBS_DOWN, + ) + + +@pytest.mark.django_db +def test_similar_image_feedback_model_str(user, image_factory): + """Test the string representation of the feedback model.""" + source_image = image_factory(public=True) + similar_image = image_factory(public=True) + + feedback = SimilarImageFeedback.objects.create( + image=source_image, + similar_image=similar_image, + user=user, + feedback=SimilarImageFeedback.THUMBS_UP, + ) + + expected = f"{user.username}: {source_image.isic_id} -> {similar_image.isic_id} (up)" + assert str(feedback) == expected