Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 28 additions & 3 deletions isic/core/api/image.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from typing import Any

from django.conf import settings
from django.contrib import messages
from django.core.cache import cache
from django.http.request import HttpRequest
from django.shortcuts import get_object_or_404
Expand All @@ -12,7 +13,7 @@
from sentry_sdk import set_tag

from isic.core.models import Image
from isic.core.pagination import CursorPagination, qs_with_hardcoded_count
from isic.core.pagination import DynamicOrderCursorPagination, qs_with_hardcoded_count
from isic.core.permissions import get_visible_objects
from isic.core.search import facets, get_elasticsearch_client
from isic.core.serializers import SearchQueryIn
Expand Down Expand Up @@ -84,7 +85,7 @@ def resolve_metadata(image: Image) -> dict:
@router.get(
"/", response=list[ImageOut], summary="Return a list of images.", include_in_schema=True
)
@paginate(CursorPagination, ordering=Image._meta.ordering)
@paginate(DynamicOrderCursorPagination, ordering=Image._meta.ordering)
def image_list(request: HttpRequest):
qs = get_visible_objects(request.user, "core.view_image", default_qs)

Expand All @@ -106,7 +107,7 @@ def image_list(request: HttpRequest):
description=render_to_string("core/swagger_image_search_description.html"),
include_in_schema=True,
)
@paginate(CursorPagination, ordering=Image._meta.ordering)
@paginate(DynamicOrderCursorPagination, ordering=Image._meta.ordering)
def image_search(request: HttpRequest, search: SearchQueryIn = Query(...)):
try:
qs = search.to_queryset(user=request.user, qs=default_qs)
Expand Down Expand Up @@ -209,3 +210,27 @@ def image_similar(
similar_qs = image.similar_images().select_related("accession__cohort")
similar_qs = get_visible_objects(request.user, "core.view_image", similar_qs)
return similar_qs[:limit]


class SetPinned(Schema):
pinned: bool

model_config = {"extra": "forbid"}


@router.post(
"/{id}/set-pinned/",
response={200: None, 400: dict, 403: dict},
include_in_schema=False,
)
def image_set_pinned(request, id: int, payload: SetPinned):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this just be a form endpoint instead? That's a real question, not a suggestion.

@danlamanna Thoughts?

if not request.user.is_staff:
return 403, {"error": "You do not have permission to pin or unpin this image."}

qs = get_visible_objects(request.user, "core.view_image", Image.objects.all())
image = get_object_or_404(qs.distinct(), id=id)
image.pinned = payload.pinned
image.save()
action = "pinned" if payload.pinned else "unpinned"
messages.add_message(request, messages.SUCCESS, f"Image {action}.")
return 200, None
17 changes: 17 additions & 0 deletions isic/core/migrations/0042_pinned_images.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 5.2.15 on 2026-06-09 14:59

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("core", "0041_alter_collection_creator_and_more"),
]

operations = [
migrations.AddField(
model_name="image",
name="pinned",
field=models.BooleanField(default=False),
),
]
2 changes: 2 additions & 0 deletions isic/core/models/image.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ class Meta(CreationSortedTimeStampedModel.Meta):
# index is used because public is filtered in every permissions check
public = models.BooleanField(default=False, db_index=True)

pinned = models.BooleanField(default=False)

shares: models.ManyToManyField[User, ImageShare] = models.ManyToManyField(
User, through="ImageShare", through_fields=("image", "grantee")
)
Expand Down
48 changes: 38 additions & 10 deletions isic/core/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from typing import Any
from urllib import parse

from django.db.models import Q
from django.db.models.query import QuerySet
from django.http.request import HttpRequest
from ninja import Field, Schema
Expand Down Expand Up @@ -142,13 +143,22 @@ def paginate_queryset(
queryset = queryset.order_by(*_reverse_order(order))

if cursor.position is not None:
is_reversed = order[0].startswith("-")
order_attr = order[0].lstrip("-")

if cursor.reverse != is_reversed:
queryset = queryset.filter(**{f"{order_attr}__lt": cursor.position})
else:
queryset = queryset.filter(**{f"{order_attr}__gt": cursor.position})
position_values = cursor.position.split("|")

# Build Q object for comparison across all ordering fields
q_obj = Q()
for i, (field, pos_val) in enumerate(zip(order, position_values, strict=True)):
is_reversed = field.startswith("-")
field_name = field.lstrip("-")
comparison = "__gt" if cursor.reverse == is_reversed else "__lt"
field_condition = Q(**{f"{field_name}{comparison}": pos_val})
for j in range(i):
prev_field = order[j]
prev_field_name = prev_field.lstrip("-")
field_condition &= Q(**{prev_field_name: position_values[j]})
q_obj |= field_condition

queryset = queryset.filter(q_obj)

# If we have an offset cursor then offset the entire page by that amount.
# We also always fetch an extra item in order to determine if there is a
Expand Down Expand Up @@ -347,6 +357,24 @@ def previous_link( # noqa: PLR0913
return self._encode_cursor(cursor, base_url)

def _get_position_from_instance(self, instance, ordering):
field_name = ordering[0].lstrip("-")
attr = instance[field_name] if isinstance(instance, dict) else getattr(instance, field_name)
return str(attr)
values = []
for field in ordering:
fname = field.lstrip("-")
attr = instance[fname] if isinstance(instance, dict) else getattr(instance, fname)
values.append(str(attr))
return "|".join(values)


class DynamicOrderCursorPagination(CursorPagination):
def paginate_queryset(
self, queryset: QuerySet, pagination: any, request: HttpRequest, **params
) -> dict:
order_by = request.GET.get("order_by")
if order_by:
field_names = [field.name for field in queryset.model._meta.get_fields()]
self.ordering = [
field_name.strip()
for field_name in order_by.split(",")
if (field_name if field_name[0] != "-" else field_name[1:]) in field_names
]
return super().paginate_queryset(queryset, pagination, request, **params)
41 changes: 41 additions & 0 deletions isic/core/templates/core/image_detail/actions_menu.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<button class="btn" popovertarget="image-detail-actions">
Actions
<i class="ri-arrow-down-s-line"></i>
</button>
<ul class="dropdown dropdown-end menu w-max overflow-visible rounded-box bg-base-100 shadow-sm"
popover id="image-detail-actions" x-data=imageActions()>

{% if request.user.is_staff %}
{% if image.pinned %}
<li>
<button @click="setPinned(false)">Unpin Image</button>
</li>
{% else %}
<li>
<button @click="setPinned(true)">Pin Image</button>
</li>
{% endif %}
{% endif %}
</ul>

<script type="text/javascript">
function imageActions() {
return {
errorMessage: '',
async setPinned(pinned) {
try {
const resp = await axios.post('/api/v2/images/{{ image.pk }}/set-pinned/', { pinned }, {
headers: {
'Content-Type': 'application/json',
'X-CSRFToken': '{{ csrf_token }}'
}
})
window.location.reload();
} catch (error) {
alert('Something went wrong, try again.');
throw error;
}
}
}
}
</script>
17 changes: 11 additions & 6 deletions isic/core/templates/core/image_detail/base.html
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,18 @@

{% block content %}
<div class="space-y-4">
<div class="flex items-center gap-3">
<div class="heading-1">{{image.isic_id}}</div>
<div class="flex justify-between w-full">
<div class="flex items-center gap-3">
<div class="heading-1">{{image.isic_id}}</div>

{% if image.public %}
<span class="badge badge-success">Public</span>
{% else %}
<span class="badge badge-warning">Private</span>
{% if image.public %}
<span class="badge badge-success">Public</span>
{% else %}
<span class="badge badge-warning">Private</span>
{% endif %}
</div>
{% if request.user.is_staff %}
{% include 'core/image_detail/actions_menu.html' %}
{% endif %}
</div>

Expand Down
70 changes: 70 additions & 0 deletions isic/core/tests/test_image_pin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
from django.urls import reverse
from playwright.sync_api import expect
import pytest
from pytest_lazy_fixtures import lf


@pytest.mark.django_db
@pytest.mark.parametrize(
("client_", "expected_status"),
[
(lf("client"), 403),
(lf("authenticated_client"), 403),
(lf("staff_client"), 200),
],
ids=["anonymous", "authenticated", "staff"],
)
def test_core_api_image_set_pinned_permissions(client_, expected_status, image_factory):
image = image_factory(public=True, pinned=False)
r = client_.post(
reverse("api:image_set_pinned", kwargs={"id": image.pk}),
{"pinned": True},
content_type="application/json",
)
assert r.status_code == expected_status

image.refresh_from_db()
assert image.pinned == (expected_status == 200)


@pytest.mark.django_db
def test_core_api_image_sort_by_pinned(image_factory, authenticated_client):
image_1 = image_factory(public=True, pinned=False)
image_2 = image_factory(public=True, pinned=True)
order_by = "-pinned,created"

# List endpoint
r = authenticated_client.get(reverse("api:image_list"), data={"order_by": order_by})
ordered_ids = [image.get("isic_id") for image in r.json().get("results")]
assert ordered_ids == [image_2.isic_id, image_1.isic_id]

# Search endpoint
r = authenticated_client.get(reverse("api:image_search"), data={"order_by": order_by})
ordered_ids = [image.get("isic_id") for image in r.json().get("results")]
assert ordered_ids == [image_2.isic_id, image_1.isic_id]


@pytest.mark.playwright
def test_image_pin_unpin(image_factory, staff_authenticated_page):
page = staff_authenticated_page
image_id = image_factory().isic_id
page.goto(reverse("core/image-detail", args=[image_id]))

# Pin the image
page.get_by_role("button", name="Actions").click()
page.get_by_role("button", name="Pin image").click()
page.wait_for_url(f"**{reverse('core/image-detail', args=[image_id])}")
expect(page.get_by_text("image pinned.")).to_be_visible()

# Unpin button should now be present
page.get_by_role("button", name="Actions").click()
expect(page.get_by_role("button", name="Unpin image")).to_be_visible()

# Unpin the image
page.get_by_role("button", name="Unpin image").click()
page.wait_for_url(f"**{reverse('core/image-detail', args=[image_id])}")
expect(page.get_by_text("image unpinned.")).to_be_visible()

# Pin button should be back
page.get_by_role("button", name="Actions").click()
expect(page.get_by_role("button", name="Pin image")).to_be_visible()
5 changes: 3 additions & 2 deletions isic/core/views/images.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ def image_browser(request):
collections=collections,
)
qs: QuerySet[Image] = Image.objects.none()
order_by = ("-pinned", "created")

if search_form.is_valid():
qs = search_form.results
Expand All @@ -168,9 +169,9 @@ def image_browser(request):
index=settings.ISIC_ELASTICSEARCH_IMAGES_INDEX,
body={"query": es_query},
)["count"]
qs = qs_with_hardcoded_count(qs, ("created",), es_count)
qs = qs_with_hardcoded_count(qs, order_by, es_count)

paginator = CursorPagination(ordering=("created",))
paginator = CursorPagination(ordering=order_by)
cursor_input = CursorPagination.Input(
limit=request.GET.get("limit", 30), cursor=request.GET.get("cursor")
)
Expand Down