Skip to content

perf: optimize queries for SliderNumericFilter #1160

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.

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
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
40 changes: 17 additions & 23 deletions src/unfold/contrib/filters/admin/numeric_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from django.contrib.admin.options import ModelAdmin
from django.contrib.admin.views.main import ChangeList
from django.core.validators import EMPTY_VALUES
from django.db.models import Max, Min, Model, QuerySet
from django.db.models import Count, Max, Min, Model, QuerySet
from django.db.models.fields import (
AutoField,
DecimalField,
Expand Down Expand Up @@ -146,15 +146,13 @@ def __init__(
self.q = model_admin.get_queryset(request)

def choices(self, changelist: ChangeList) -> tuple[dict[str, Any], ...]:
total = self.q.all().count()
min_value = self.q.all().aggregate(min=Min(self.parameter_name)).get("min", 0)

if total > 1:
max_value = (
self.q.all().aggregate(max=Max(self.parameter_name)).get("max", 0)
)
else:
max_value = None
aggregates = self.q.aggregate(
min=Min(self.parameter_name),
max=Max(self.parameter_name),
total=Count("pk"),
)
min_value = aggregates.get("min", 0)
max_value = aggregates.get("max", 0) if aggregates["total"] > 1 else None

if isinstance(self.field, (FloatField, DecimalField)):
decimals = self.MAX_DECIMALS
Expand All @@ -163,6 +161,9 @@ def choices(self, changelist: ChangeList) -> tuple[dict[str, Any], ...]:
decimals = 0
step = self.STEP if self.STEP else 1

value_from = self.used_parameters.get(self.parameter_name + "_from", min_value)
value_to = self.used_parameters.get(self.parameter_name + "_to", max_value)

return (
{
"decimals": decimals,
Expand All @@ -171,26 +172,19 @@ def choices(self, changelist: ChangeList) -> tuple[dict[str, Any], ...]:
"request": self.request,
"min": min_value,
"max": max_value,
"value_from": self.used_parameters.get(
self.parameter_name + "_from", min_value
),
"value_to": self.used_parameters.get(
self.parameter_name + "_to", max_value
),
"value_from": value_from,
"value_to": value_to,
"form": self.form_class(
name=self.parameter_name,
data={
self.parameter_name + "_from": self.used_parameters.get(
self.parameter_name + "_from", min_value
),
self.parameter_name + "_to": self.used_parameters.get(
self.parameter_name + "_to", max_value
),
self.parameter_name + "_from": value_from,
self.parameter_name + "_to": value_to,
},
),
},
)

def _get_min_step(self, precision: int) -> float:
@staticmethod
def _get_min_step(precision: int) -> float:
result_format = f"{{:.{precision - 1}f}}"
return float(result_format.format(0) + "1")
17 changes: 17 additions & 0 deletions tests/server/example/migrations/0003_user_age.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Generated by Django 4.2.18 on 2025-03-06 05:24

from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("example", "0002_user_content_type"),
]

operations = [
migrations.AddField(
model_name="user",
name="age",
field=models.PositiveSmallIntegerField(blank=True, null=True),
),
]
1 change: 1 addition & 0 deletions tests/server/example/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ class User(AbstractUser):
content_type = models.ForeignKey(
"contenttypes.ContentType", on_delete=models.CASCADE, null=True, blank=True
)
age = models.PositiveSmallIntegerField(null=True, blank=True)
97 changes: 77 additions & 20 deletions tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.contrib.admin.templatetags.admin_list import admin_list_filter
from django.contrib.auth import get_user_model

from src.unfold.contrib.filters.admin.numeric_filters import SliderNumericFilter
from unfold.contrib.filters.admin.autocomplete_filters import (
AutocompleteSelectFilter,
AutocompleteSelectMultipleFilter,
Expand All @@ -11,49 +12,105 @@

@pytest.mark.django_db
def test_filters_field_text_filter(admin_request, user_model_admin, user_changelist):
filter = FieldTextFilter(
# Arrange
User = get_user_model()
user_field = "username"

# Act
admin_filter = FieldTextFilter(
request=admin_request,
params={"username": "test"},
model=get_user_model(),
params={user_field: "test"},
model=User,
model_admin=user_model_admin,
field="username",
field_path="username",
field=User._meta.get_field(user_field),
field_path=user_field,
)

assert "id_username__icontains" in admin_list_filter(user_changelist, filter)
# Assert
assert f"id_{user_field}__icontains" in admin_list_filter(
user_changelist, admin_filter
)


@pytest.mark.django_db
def test_filters_autocomplete_select_multiple_filter(
admin_request, user_model_admin, user_changelist
):
user_model = get_user_model()
# Arrange
User = get_user_model()
user_field = "content_type"

filter = AutocompleteSelectMultipleFilter(
# Act
admin_filter = AutocompleteSelectMultipleFilter(
request=admin_request,
params={"content_type": "test"},
model=get_user_model(),
params={user_field: "test"},
model=User,
model_admin=user_model_admin,
field=user_model._meta.get_field("content_type"),
field_path="content_type",
field=User._meta.get_field(user_field),
field_path=user_field,
)

assert "id_content_type__id__exact" in admin_list_filter(user_changelist, filter)
# Assert
assert f"id_{user_field}__id__exact" in admin_list_filter(
user_changelist, admin_filter
)


@pytest.mark.django_db
def test_filters_autocomplete_select_filter(
admin_request, user_model_admin, user_changelist
):
user_model = get_user_model()
# Arrange
User = get_user_model()
user_field = "content_type"

# Act
admin_filter = AutocompleteSelectFilter(
request=admin_request,
params={user_field: "test"},
model=User,
model_admin=user_model_admin,
field=User._meta.get_field(user_field),
field_path=user_field,
)

# Assert
assert f"id_{user_field}__id__exact" in admin_list_filter(
user_changelist, admin_filter
)


@pytest.mark.django_db
def test_filters_slider_numeric_filter(
admin_request, user_model_admin, user_changelist
):
# Arrange
User = get_user_model()
user_field = "age"
min = 10
max = 20
User.objects.create_user(username="test_child", age=min)
User.objects.create_user(username="test_adult", age=max)
value_from = 18
value_to = 30

filter = AutocompleteSelectFilter(
# Act
admin_filter = SliderNumericFilter(
request=admin_request,
params={"content_type": "test"},
model=get_user_model(),
params={f"{user_field}_from": value_from, f"{user_field}_to": value_to},
model=User,
model_admin=user_model_admin,
field=user_model._meta.get_field("content_type"),
field_path="content_type",
field=User._meta.get_field(user_field),
field_path=user_field,
)
choices = admin_filter.choices(user_changelist)[0]
filtered_list = admin_list_filter(user_changelist, admin_filter)

assert "id_content_type__id__exact" in admin_list_filter(user_changelist, filter)
# Assert
assert choices.get("min") == min
assert choices.get("max") == max
assert choices.get("value_from") == value_from
assert choices.get("value_to") == value_to
assert choices.get("request") == admin_request
assert f"{user_field}_from" in filtered_list
assert f"{user_field}_to" in filtered_list