From 592edd780a0f1bf78ea57854fd817d3391d0eb06 Mon Sep 17 00:00:00 2001 From: Dan LaManna Date: Fri, 18 Apr 2025 03:12:44 -0400 Subject: [PATCH] Add page to display cohort derived collections --- isic/core/templates/core/collection_list.html | 7 ++- .../tests/test_collection_list_browser.py | 51 +++++++++++++++++++ isic/core/views/collections.py | 13 ++++- isic/ingest/models/cohort.py | 6 +++ .../ingest/partials/cohort_actions.html | 4 ++ 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/isic/core/templates/core/collection_list.html b/isic/core/templates/core/collection_list.html index d0215ea7..9f7f196f 100644 --- a/isic/core/templates/core/collection_list.html +++ b/isic/core/templates/core/collection_list.html @@ -25,7 +25,12 @@ {% block content %}
-
Collections ({{ page.paginator.count|intcomma }})
+ {% if cohort %} +
Collections derived from {{ cohort.name }} ({{ page.paginator.count|intcomma }})
+ Show all collections + {% else %} +
Collections ({{ page.paginator.count|intcomma }})
+ {% endif %}
{% if request.user.is_authenticated %} diff --git a/isic/core/tests/test_collection_list_browser.py b/isic/core/tests/test_collection_list_browser.py index b95fa569..e269c4da 100644 --- a/isic/core/tests/test_collection_list_browser.py +++ b/isic/core/tests/test_collection_list_browser.py @@ -158,3 +158,54 @@ def test_collection_list_mobile( expect( page.locator("tbody tr").filter(has_text=collection_private.name).get_by_text("Private") ).to_be_visible() + + +@pytest.mark.playwright +def test_collection_list_cohort_filter( + staff_authenticated_page, + authenticated_page, + cohort_factory, + accession_factory, + collection_factory, + image_factory, +): + page = staff_authenticated_page + + cohort = cohort_factory() + derived_collection = collection_factory(public=True, pinned=False, locked=False) + unrelated_collection = collection_factory(public=True, pinned=False, locked=False) + + for _ in range(2): + add_images_to_collection( + collection=derived_collection, + image=image_factory(public=True, accession=accession_factory(cohort=cohort)), + ) + + add_images_to_collection(collection=unrelated_collection, image=image_factory(public=True)) + + _refresh_collection_counts() + + page.goto(reverse("ingest/cohort-detail", args=[cohort.pk])) + page.get_by_role("button", name="Actions").click() + page.get_by_role("link", name="View Derived Collections").click() + page.wait_for_url("**/collections/?*") + + expect(page.get_by_text(f"Collections derived from {cohort.name}")).to_be_visible() + expect(page.get_by_role("link", name=derived_collection.name)).to_be_visible() + expect(page.get_by_role("link", name=unrelated_collection.name)).not_to_be_visible() + + # Clearing the filter brings back the collections that aren't derived from the cohort + page.get_by_role("link", name="Show all collections").click() + page.wait_for_load_state("networkidle") + expect(page.get_by_text(f"Collections derived from {cohort.name}")).not_to_be_visible() + expect(page.get_by_role("link", name=unrelated_collection.name)).to_be_visible() + + # Non-staff users can't narrow by cohort, so the parameter is ignored + non_staff_page = authenticated_page + non_staff_page.goto( + reverse("core/collection-list", query={"cohort": cohort.pk, "magic_filter": "all"}) + ) + expect( + non_staff_page.get_by_text(f"Collections derived from {cohort.name}") + ).not_to_be_visible() + expect(non_staff_page.get_by_role("link", name=unrelated_collection.name)).to_be_visible() diff --git a/isic/core/views/collections.py b/isic/core/views/collections.py index 4353947c..8b966720 100644 --- a/isic/core/views/collections.py +++ b/isic/core/views/collections.py @@ -24,7 +24,7 @@ from isic.core.services.collection import create_collection, update_collection from isic.core.utils.csv import EscapingDictWriter from isic.core.utils.http import Echo -from isic.ingest.models import Contributor +from isic.ingest.models import Cohort, Contributor @login_required @@ -194,6 +194,16 @@ def collection_list(request: HttpRequest) -> HttpResponse: pinned_filter = request.GET.get("pinned_filter", "all") exclude_empty = request.GET.get("exclude_empty", "1") == "1" + cohort_filter = request.GET.get("cohort", "") + cohort = None + if request.user.is_staff and cohort_filter.isdigit(): + cohort = get_object_or_404(Cohort, pk=cohort_filter) + collections = collections.filter( + pk__in=Collection.images.through.objects.filter(image__accession__cohort=cohort).values( + "collection_id" + ) + ) + if magic_filter == "only": collections = collections.magic() elif magic_filter == "exclude": @@ -243,5 +253,6 @@ def collection_list(request: HttpRequest) -> HttpResponse: "magic_filter": magic_filter, "pinned_filter": pinned_filter, "exclude_empty": exclude_empty, + "cohort": cohort, }, ) diff --git a/isic/ingest/models/cohort.py b/isic/ingest/models/cohort.py index 48219554..b7f2767e 100644 --- a/isic/ingest/models/cohort.py +++ b/isic/ingest/models/cohort.py @@ -77,6 +77,12 @@ def __str__(self) -> str: def get_absolute_url(self): return reverse("ingest/cohort-detail", args=[self.id]) + @property + def derived_collections_url(self) -> str: + # magic_filter is overridden because the collection list excludes magic collections by + # default, which would hide the cohort's own collection. + return reverse("core/collection-list", query={"cohort": self.id, "magic_filter": "all"}) + @property def num_lesions(self): return self.accessions.exclude(lesion=None).values("lesion__id").distinct().count() diff --git a/isic/ingest/templates/ingest/partials/cohort_actions.html b/isic/ingest/templates/ingest/partials/cohort_actions.html index 86d59caf..087aa576 100644 --- a/isic/ingest/templates/ingest/partials/cohort_actions.html +++ b/isic/ingest/templates/ingest/partials/cohort_actions.html @@ -26,4 +26,8 @@
  • Publish Cohort
    • +
    +
  • + View Derived Collections +