From 55ff5f25964255374b49b7e5c5a0c199529f9317 Mon Sep 17 00:00:00 2001 From: Morten Madsen Lyngstad Date: Wed, 16 Jul 2025 12:55:57 +0200 Subject: [PATCH 1/2] Implement custom header rendering for sortable tables and enable multi-column sorting --- src/staff/templatetags/table_header_tag.py | 80 +++++++++++++++++++ src/templates/django_tables2/header.html | 25 ++++++ .../django_tables2/tailwind_inner.html | 54 ++++--------- 3 files changed, 122 insertions(+), 37 deletions(-) create mode 100644 src/staff/templatetags/table_header_tag.py create mode 100644 src/templates/django_tables2/header.html diff --git a/src/staff/templatetags/table_header_tag.py b/src/staff/templatetags/table_header_tag.py new file mode 100644 index 00000000..6dfd3b08 --- /dev/null +++ b/src/staff/templatetags/table_header_tag.py @@ -0,0 +1,80 @@ +from django import template +from django.http import HttpRequest + +register = template.Library() + + +@register.inclusion_tag("django_tables2/header.html", takes_context=True) +def render_header(context: dict) -> dict: + url = Url(context["table"].prefixed_order_by_field, context["request"]) + for column in context["table"].columns: + sort_url, remove_sort_url, first_sort, descending = url.get_sort_url( + column.name + ) + column.ext = { + "sort_url": sort_url, + "remove_sort_url": remove_sort_url, + "first_sort": first_sort, + "descending": descending, + "next": remove_sort_url if not first_sort and descending else sort_url, + } + + return context + + +class Url: + """ + Based on code from: + https://github.com/TheRealVizard/django-table-sort/blob/main/django_table_sort/table.py + """ + + def __init__(self, sort_key_name: str, request: HttpRequest): + self.sort_key_name = sort_key_name + self.request = request + + def contains_field(self, lookups: list, field: str) -> int: + """Check if the field is in the sort lookups.""" + try: + return lookups.index(field) + except ValueError: + return -1 + + def get_sort_url(self, field: str) -> tuple[str, str, bool, bool]: + """Generate the urls to sort the table for the given field.""" + lookups = self.request.GET.copy() + removed_lookup = self.request.GET.copy() + + first_sort = True + descending = True + + if self.sort_key_name in lookups.keys(): + current_order = lookups.getlist(self.sort_key_name, []) + removed_order = current_order.copy() + position = self.contains_field(current_order, field) + if position != -1: + first_sort = False + descending = False + current_order[position] = f"-{field}" + removed_order.remove(field) + else: + position = self.contains_field(current_order, f"-{field}") + if position != -1: + first_sort = False + current_order[position] = field + removed_order.remove(f"-{field}") + else: + current_order.append(field) + lookups.setlist(self.sort_key_name, current_order) + if len(removed_order) >= 1: + removed_lookup.setlist(self.sort_key_name, removed_order) + else: + removed_lookup.pop(self.sort_key_name) + else: + lookups.setlist(self.sort_key_name, [field]) + + return ( + lookups.urlencode(), + removed_lookup.urlencode(), + first_sort, + descending, + ) diff --git a/src/templates/django_tables2/header.html b/src/templates/django_tables2/header.html new file mode 100644 index 00000000..366c8883 --- /dev/null +++ b/src/templates/django_tables2/header.html @@ -0,0 +1,25 @@ + + + {% for column in table.columns %} + + {% if column.orderable %} + + {{ column.header }} + {% if column.is_ordered %} + {% if "-" in column.order_by_alias %} + + {% else %} + + {% endif %} + {% else %} + + {% endif %} + + {% else %} + {{ column.header }} + {% endif %} + + {% endfor %} + + diff --git a/src/templates/django_tables2/tailwind_inner.html b/src/templates/django_tables2/tailwind_inner.html index 24485f46..37ab8436 100644 --- a/src/templates/django_tables2/tailwind_inner.html +++ b/src/templates/django_tables2/tailwind_inner.html @@ -1,53 +1,33 @@ {% load django_tables2 %} +{% load table_header_tag %} {% load i18n %} {% block table %} {% block table.thead %} - {% if table.show_header %} - - - {% for column in table.columns %} - - {% endfor %} - - - {% endif %} + {% if table.show_header %} + {% render_header %} + {% endif %} {% endblock table.thead %} - - {% block table.tbody %} {% for row in table.paginated_rows %} {% block table.tbody.row %} - + - {% for column, cell in row.items %} - - {% endfor %} - + {% if column.localize %} + {{ cell|localize }} + {% else %} + {{ cell|unlocalize }} + {% endif %} + {% endif %} + {% endfor %} + {% endblock table.tbody.row %} {% empty %} {% if table.empty_text %} From 2790dfdd6a02cb1c7f842b26cd13299270775036 Mon Sep 17 00:00:00 2001 From: Morten Madsen Lyngstad Date: Thu, 17 Jul 2025 11:17:38 +0200 Subject: [PATCH 2/2] Refactor sample status management Replace SampleStatusAssignment with boolean fields for marked, plucked, and isolated. Update other parts of the code which were effected by this change. --- src/genlab_bestilling/admin.py | 5 -- src/genlab_bestilling/api/views.py | 50 ++++-------- ...e_is_isolated_sample_is_marked_and_more.py | 30 +++++++ src/genlab_bestilling/models.py | 36 +-------- src/staff/tables.py | 7 ++ src/staff/templatetags/order_tags.py | 17 ++-- src/staff/views.py | 80 +++++-------------- 7 files changed, 86 insertions(+), 139 deletions(-) create mode 100644 src/genlab_bestilling/migrations/0028_sample_is_isolated_sample_is_marked_and_more.py diff --git a/src/genlab_bestilling/admin.py b/src/genlab_bestilling/admin.py index 0d58dff2..227e6064 100644 --- a/src/genlab_bestilling/admin.py +++ b/src/genlab_bestilling/admin.py @@ -23,7 +23,6 @@ Organization, Sample, SampleMarkerAnalysis, - SampleStatusAssignment, SampleType, Species, ) @@ -534,9 +533,5 @@ class AnalysisResultAdmin(ModelAdmin): ] -@admin.register(SampleStatusAssignment) -class SampleStatusAssignmentAdmin(ModelAdmin): ... - - @admin.register(IsolationMethod) class IsolationMethodAdmin(ModelAdmin): ... diff --git a/src/genlab_bestilling/api/views.py b/src/genlab_bestilling/api/views.py index 76dac513..9ea27c65 100644 --- a/src/genlab_bestilling/api/views.py +++ b/src/genlab_bestilling/api/views.py @@ -1,7 +1,7 @@ import uuid from django.db import transaction -from django.db.models import Exists, OuterRef, QuerySet +from django.db.models import QuerySet from django.http import HttpResponse from django.views import View from drf_spectacular.utils import extend_schema @@ -33,7 +33,6 @@ Marker, Sample, SampleMarkerAnalysis, - SampleStatusAssignment, SampleType, Species, ) @@ -98,9 +97,9 @@ class SampleViewset(ModelViewSet): "project": "Projectnumber", "isolation_method": "Isolation Method", "qiagen_number": "Qiagen#", - "marked": "Marked", - "plucked": "Plucked", - "isolated": "Isolated", + "is_marked": "Marked", + "is_plucked": "Plucked", + "is_isolated": "Isolated", "station": "Station", "placement_in_fridge": "Placement in fridge", "delivered_to_lab": "Delivered to lab", @@ -128,9 +127,9 @@ class SampleViewset(ModelViewSet): "type.name", "isolation_method", "qiagen_number", - "marked", - "plucked", - "isolated", + "is_marked", + "is_plucked", + "is_isolated", ], "Elvemusling": [ "genlab_id", @@ -146,9 +145,9 @@ class SampleViewset(ModelViewSet): "isolation_method", "qiagen_number", "placement_in_fridge", - "marked", - "plucked", - "isolated", + "is_marked", + "is_plucked", + "is_isolated", ], "Terrestrisk": [ "genlab_id", @@ -161,9 +160,9 @@ class SampleViewset(ModelViewSet): "order", "analysis_orders", "notes", - "marked", - "plucked", - "isolated", + "is_marked", + "is_plucked", + "is_isolated", "isolation_method", "qiagen_number", ], @@ -179,9 +178,9 @@ class SampleViewset(ModelViewSet): "order", "analysis_orders", "notes", - "marked", - "plucked", - "isolated", + "is_marked", + "is_plucked", + "is_isolated", "isolation_method", "qiagen_number", ], @@ -199,23 +198,6 @@ def get_queryset(self) -> QuerySet: "order__genrequest__area", "location", ) - .annotate( - is_marked=Exists( - SampleStatusAssignment.objects.filter( - sample=OuterRef("pk"), status="marked" - ) - ), - is_plucked=Exists( - SampleStatusAssignment.objects.filter( - sample=OuterRef("pk"), status="plucked" - ) - ), - is_isolated=Exists( - SampleStatusAssignment.objects.filter( - sample=OuterRef("pk"), status="isolated" - ) - ), - ) .order_by("genlab_id", "type") ) diff --git a/src/genlab_bestilling/migrations/0028_sample_is_isolated_sample_is_marked_and_more.py b/src/genlab_bestilling/migrations/0028_sample_is_isolated_sample_is_marked_and_more.py new file mode 100644 index 00000000..7204ccb1 --- /dev/null +++ b/src/genlab_bestilling/migrations/0028_sample_is_isolated_sample_is_marked_and_more.py @@ -0,0 +1,30 @@ +# Generated by Django 5.2.3 on 2025-07-17 08:16 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("genlab_bestilling", "0027_alter_isolationmethod_name"), + ] + + operations = [ + migrations.AddField( + model_name="sample", + name="is_isolated", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="sample", + name="is_marked", + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name="sample", + name="is_plucked", + field=models.BooleanField(default=False), + ), + migrations.DeleteModel( + name="SampleStatusAssignment", + ), + ] diff --git a/src/genlab_bestilling/models.py b/src/genlab_bestilling/models.py index 47f646c3..892c3a9d 100644 --- a/src/genlab_bestilling/models.py +++ b/src/genlab_bestilling/models.py @@ -638,6 +638,10 @@ class Sample(models.Model): year = models.IntegerField() notes = models.TextField(null=True, blank=True) + is_marked = models.BooleanField(default=False) + is_plucked = models.BooleanField(default=False) + is_isolated = models.BooleanField(default=False) + # "Merknad" in the Excel sheet. internal_note = models.TextField(null=True, blank=True) pop_id = models.CharField(max_length=150, null=True, blank=True) @@ -770,38 +774,6 @@ def generate_genlab_id(self, commit: bool = True) -> str: # assignee (one or plus?) -class SampleStatusAssignment(models.Model): - class SampleStatus(models.TextChoices): - MARKED = "marked", _("Marked") - PLUCKED = "plucked", _("Plucked") - ISOLATED = "isolated", _("Isolated") - - sample = models.ForeignKey( - f"{an}.Sample", - on_delete=models.CASCADE, - related_name="sample_status_assignments", - ) - status = models.CharField( - choices=SampleStatus.choices, - null=True, - blank=True, - verbose_name="Sample status", - help_text="The status of the sample in the lab", - ) - order = models.ForeignKey( - f"{an}.Order", - on_delete=models.CASCADE, - related_name="sample_status_assignments", - null=True, - blank=True, - ) - - assigned_at = models.DateTimeField(auto_now_add=True) - - class Meta: - unique_together = ("sample", "status", "order") - - class SampleIsolationMethod(models.Model): sample = models.ForeignKey( f"{an}.Sample", diff --git a/src/staff/tables.py b/src/staff/tables.py index ee4d8523..e47476b3 100644 --- a/src/staff/tables.py +++ b/src/staff/tables.py @@ -251,18 +251,21 @@ class SampleStatusTable(tables.Table): orderable=True, yesno="✔,-", default=False, + accessor="is_marked", ) plucked = tables.BooleanColumn( verbose_name="Plucked", orderable=True, yesno="✔,-", default=False, + accessor="is_plucked", ) isolated = tables.BooleanColumn( verbose_name="Isolated", orderable=True, yesno="✔,-", default=False, + accessor="is_isolated", ) class Meta: @@ -270,6 +273,9 @@ class Meta: fields = [ "checked", "genlab_id", + "marked", + "plucked", + "isolated", "internal_note", "isolation_method", "type", @@ -284,6 +290,7 @@ class Meta: "internal_note", "isolation_method", ] + order_by = () def render_checked(self, record: Any) -> str: return mark_safe( # noqa: S308 diff --git a/src/staff/templatetags/order_tags.py b/src/staff/templatetags/order_tags.py index 828e2b74..5b8d8e3a 100644 --- a/src/staff/templatetags/order_tags.py +++ b/src/staff/templatetags/order_tags.py @@ -2,7 +2,7 @@ from django.db import models from capps.users.models import User -from genlab_bestilling.models import Area, Order, SampleStatusAssignment +from genlab_bestilling.models import Area, Order from ..tables import ( AssignedOrderTable, @@ -168,12 +168,15 @@ def assigned_orders_table(context: dict) -> dict: ) .select_related("genrequest") .annotate( - isolated_sample_count=models.Count( - "sample_status_assignments", - distinct=True, - filter=models.Q( - sample_status_assignments__status=SampleStatusAssignment.SampleStatus.ISOLATED, - ), + isolated_sample_count=models.Case( + models.When( + extractionorder__isnull=False, + then=models.Count( + "extractionorder__samples", + filter=models.Q(extractionorder__samples__is_isolated=True), + distinct=True, + ), + ) ), sample_count=models.Case( models.When( diff --git a/src/staff/views.py b/src/staff/views.py index 25aad648..aae66779 100644 --- a/src/staff/views.py +++ b/src/staff/views.py @@ -1,10 +1,9 @@ -from collections import defaultdict from typing import Any from django.contrib import messages from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin from django.db import models -from django.db.models import Count, Exists, OuterRef +from django.db.models import Count from django.forms import Form from django.http import HttpRequest, HttpResponse, HttpResponseRedirect, JsonResponse from django.shortcuts import get_object_or_404 @@ -28,7 +27,6 @@ Sample, SampleIsolationMethod, SampleMarkerAnalysis, - SampleStatusAssignment, ) from nina.models import Project from shared.views import ActionView @@ -318,7 +316,7 @@ class SampleDetailView(StaffMixin, DetailView): model = Sample -class SampleLabView(StaffMixin, TemplateView): +class SampleLabView(StaffMixin, SingleTableMixin, TemplateView): disable_pagination = False template_name = "staff/sample_lab.html" table_class = SampleStatusTable @@ -328,34 +326,16 @@ def get_order(self) -> ExtractionOrder: self._order = get_object_or_404(ExtractionOrder, pk=self.kwargs["pk"]) return self._order - def get_data(self) -> list[Sample]: + def get_table_data(self) -> list[Sample]: order = self.get_order() samples = Sample.objects.filter(order=order, genlab_id__isnull=False) - sample_status = SampleStatusAssignment.SampleStatus.choices - # Fetch all SampleStatusAssignment entries related to the current order - sample_assignments = SampleStatusAssignment.objects.filter(order_id=order.id) - - # Build a lookup: {sample_id: set of status names} - # This allows us to check status presence without querying per sample - sample_status_map = defaultdict(set) - for assignment in sample_assignments: - name = str(assignment.status) - - sample_status_map[assignment.sample_id].add(name) - - # Annotate each sample instance with boolean flags per status - # Equivalent to: sample.status_name = True/False - # based on whether the sample has that status for sample in samples: sample.selected_isolation_method = ( sample.isolation_method.first() if sample.isolation_method.exists() else None ) - status_names = sample_status_map.get(sample.id, set()) - for status, _i in sample_status: - setattr(sample, status, status in status_names) return samples @@ -371,14 +351,13 @@ def get_isolation_methods(self) -> list[str]: ) def get_base_fields(self) -> list[str]: - return [v for v, _ in SampleStatusAssignment.SampleStatus.choices] + return ["marked", "plucked", "isolated"] def get_context_data(self, **kwargs: Any) -> dict[str, Any]: context = super().get_context_data(**kwargs) context["order"] = self.get_order() context["statuses"] = self.get_base_fields() context["isolation_methods"] = self.get_isolation_methods() - context["table"] = self.table_class(data=self.get_data()) return context @@ -402,7 +381,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse: samples = Sample.objects.filter(id__in=selected_ids) if status_name: - self.assign_status_to_samples(samples, status_name, order, request) + self.assign_status_to_samples(samples, status_name, request) if status_name == "isolated": # Cannot use "samples" here # because we need to check all samples in the order @@ -415,39 +394,27 @@ def assign_status_to_samples( self, samples: models.QuerySet, status_name: str, - order: ExtractionOrder, request: HttpRequest, ) -> None: - statuses = SampleStatusAssignment.SampleStatus.choices + valid_statuses = ["marked", "plucked", "isolated"] - # Check if the provided status exists - if status_name not in [k for k, _ in statuses]: + if status_name not in valid_statuses: messages.error(request, f"Status '{status_name}' is not valid.") - return HttpResponseRedirect(self.get_success_url()) + return - # Get the index of the target status - status_weight = next( - i for i, (name, _) in enumerate(statuses) if name == status_name - ) + update_fields = [] - # Slice the list up to that index (inclusive) and extract only the names - statuses_to_apply = [name for name, _ in statuses[: status_weight + 1]] + if status_name == "marked": + update_fields.append("is_marked") + samples.update(is_marked=True) - # Apply status assignments - assignments = [] - for sample in samples: - for status in statuses_to_apply: - assignment = SampleStatusAssignment( - sample=sample, - status=status, - order=order, - ) - assignments.append(assignment) + elif status_name == "plucked": + update_fields.extend(["is_marked", "is_plucked"]) + samples.update(is_marked=True, is_plucked=True) - SampleStatusAssignment.objects.bulk_create( - assignments, - ignore_conflicts=True, - ) + elif status_name == "isolated": + update_fields.extend(["is_marked", "is_plucked", "is_isolated"]) + samples.update(is_marked=True, is_plucked=True, is_isolated=True) messages.success( request, f"{samples.count()} samples updated with status '{status_name}'." @@ -456,16 +423,7 @@ def assign_status_to_samples( # Checks if all samples in the order are isolated # If they are, it updates the order status to completed def check_all_isolated(self, samples: models.QuerySet) -> None: - samples_with_flag = samples.annotate( - has_isolated=Exists( - SampleStatusAssignment.objects.filter( - sample=OuterRef("pk"), - status=SampleStatusAssignment.SampleStatus.ISOLATED, - ) - ) - ) - - if not samples_with_flag.filter(has_isolated=False).exists(): + if not samples.filter(is_isolated=False).exists(): self.get_order().to_next_status() messages.success( self.request,
- {% if column.orderable %} - {% comment%} - If the column is orderable, two small arrows will show next to the column name to signal that it can be sorted. - {% endcomment%} - - {{ column.header }} - - - {% else %} - {{ column.header }} - {% endif %} -
- {% if column.localize == None %} - {{ cell }} - {% else %} - {% if column.localize %} - {{ cell|localize }} + {% for column, cell in row.items %} + + {% if column.localize == None %} + {{ cell }} {% else %} - {{ cell|unlocalize }} - {% endif %} - {% endif %}