Skip to content

462 refactor sample status filtering 2 #468

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aastabk merged 3 commits into from
Jul 28, 2025
Merged

462 refactor sample status filtering 2 #468

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
46da9dc
Filter on marked, plucked and isolated
aastabk Jul 28, 2025
cc61888
Remove prints
aastabk Jul 28, 2025
9788395
Reuse filter method
aastabk Jul 28, 2025
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
67 changes: 34 additions & 33 deletions src/staff/filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,21 +266,35 @@ class SampleStatusWidget(forms.Select):
def __init__(self, attrs: dict[str, Any] | None = None):
choices = (
("", "---------"),
("true", "Yes"),
("false", "No"),
("marked", "Marked"),
("plucked", "Plucked"),
("isolated", "Isolated"),
Comment on lines +269 to +271
Copy link
Contributor

@emilte emilte Jul 28, 2025

Choose a reason for hiding this comment

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

Føler jeg har sett disse konstantene flere steder. Kan det struktureres og lages én sannhet en plass?

Copy link
Contributor

@mortenlyn mortenlyn Jul 29, 2025

Choose a reason for hiding this comment

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

Litt usikker på hva du mener her. De samme konstantene er brukt i SampleLabView:

class SampleLabView(StaffMixin, SingleTableMixin, SafeRedirectMixin, FilterView):
    MARKED = "marked"
    PLUCKED = "plucked"
    ISOLATED = "isolated"
    VALID_STATUSES = [MARKED, PLUCKED, ISOLATED]

Mener du å lage en egen SampleStatus klasse og definere disse konstantene der, og deretter referere dit?

Copy link
Contributor

@emilte emilte Jul 29, 2025

Choose a reason for hiding this comment

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

Det jeg tenker mest på er at hvis man skriver feil navn et sted så brekker logikken. Eller hvis noen refaktorerer en verdi senere så er det vanskelig å finne alle steder som må oppdateres. Hvis disse faktisk henger sammen bør det være eksponerbare konstanter

Copy link
Contributor

@mortenlyn mortenlyn Jul 29, 2025

Choose a reason for hiding this comment

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

Ja, skjønner! Hva tenker du er beste måten å gjøre det på? Ha en egen constants.py i staff som oppretter disse konstantene?

Copy link
Contributor

@emilte emilte Jul 29, 2025

Choose a reason for hiding this comment

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

En egen fil er kanskje lurt, da slipper man circular imports

)
super().__init__(choices=choices, attrs=attrs)


def filter_sample_status(
filter_set: Any, queryset: QuerySet, name: Any, value: str
) -> QuerySet:
if value == "marked":
# Only marked, not plucked or isolated
return queryset.filter(is_marked=True, is_plucked=False, is_isolated=False)
if value == "plucked":
# Plucked but not isolated
return queryset.filter(is_plucked=True, is_isolated=False)
if value == "isolated":
# All isolated samples, regardless of others
return queryset.filter(is_isolated=True)
return queryset


class SampleFilter(filters.FilterSet):
is_marked = filters.BooleanFilter(
label="Marked", method="filter_boolean", widget=SampleStatusWidget
)
is_plucked = filters.BooleanFilter(
label="Plucked", method="filter_boolean", widget=SampleStatusWidget
)
is_isolated = filters.BooleanFilter(
label="Isolated", method="filter_boolean", widget=SampleStatusWidget
filter_sample_status = filter_sample_status

sample_status = filters.CharFilter(
label="Sample Status",
method="filter_sample_status",
widget=SampleStatusWidget,
)

def __init__(
Expand Down Expand Up @@ -312,19 +326,9 @@ class Meta:
"year",
"location",
"pop_id",
"is_marked",
"is_plucked",
"is_isolated",
"sample_status",
)

def filter_boolean(self, queryset: QuerySet, name: str, value: Any) -> QuerySet:
val = self.data.get(name)
if str(val) == "true":
return queryset.filter(**{name: True})
if str(val) == "false":
return queryset.filter(**{name: False})
return queryset


class ExtractionPlateFilter(filters.FilterSet):
class Meta:
Expand All @@ -333,15 +337,8 @@ class Meta:


class SampleLabFilter(filters.FilterSet):
is_marked = filters.BooleanFilter(
label="Marked", method="filter_boolean", widget=SampleStatusWidget
)
is_plucked = filters.BooleanFilter(
label="Plucked", method="filter_boolean", widget=SampleStatusWidget
)
is_isolated = filters.BooleanFilter(
label="Isolated", method="filter_boolean", widget=SampleStatusWidget
)
filter_sample_status = filter_sample_status

genlab_id_min = ChoiceFilter(
label="Genlab ID (From)",
method="filter_genlab_id_range",
Expand All @@ -354,6 +351,12 @@ class SampleLabFilter(filters.FilterSet):
empty_label="Select upper bound",
)

sample_status = filters.CharFilter(
label="Sample Status",
method="filter_sample_status",
widget=SampleStatusWidget,
)

def __init__(
self,
data: dict[str, Any] | None = None,
Expand Down Expand Up @@ -406,9 +409,7 @@ class Meta:
fields = (
"genlab_id_min",
"genlab_id_max",
"is_marked",
"is_plucked",
"is_isolated",
"sample_status",
"extractions",
"isolation_method",
# "fluidigm",
Expand Down