Merge pull request #7 from heso/add-test-and-fix-init

Fix lookup_val handling on Django 4.x/5.x

Author: JobDoesburg

.github/workflows/python-publish.yml

@@ -13,11 +13,11 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
     - name: Set up Python
-      uses: actions/setup-python@v3
+      uses: actions/setup-python@v5
       with:
-        python-version: '3.9'
+        python-version: '3.10'
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip

.github/workflows/test.yml

@@ -0,0 +1,65 @@
+name: django-admin-multi-select-filter-test
+on: [push]
+env:
+  TEST_DEPENDENCIES: "tox~=4.15.1"
+jobs:
+  check-project-files:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: variables
+        run: |
+          PROJECT_FILES=(README.md pyproject.toml tox.ini)
+          ERROR=0
+      - name: test
+        run: |
+          for file in ${PROJECT_FILES[*]}; do
+            if ! [ -f $file ]; then
+            echo "$file not found in project"
+                ERROR=1
+            fi
+          done
+          exit $ERROR
+  lint:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        linters: [pyproject, lint, format]
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.13'
+          cache: 'pip'
+      - name: install-dependencies
+        run: |
+          python3 -m pip install --upgrade pip
+          pip install $TEST_DEPENDENCIES
+      - name: test
+        run: tox -e "${{ matrix.linters }}"
+  test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        minor_versions: [9, 10, 11, 12, 13]
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.${{ matrix.minor_versions }}'
+      - name: install-dependencies
+        run: |
+          python3 -m pip install --upgrade pip
+          pip install $TEST_DEPENDENCIES
+      - name: test
+        run: tox -m "py3${{ matrix.minor_versions }}"
+      - name: Upload coverage XML
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-${{ matrix.minor_versions }}
+          include-hidden-files: true
+          path: |
+            ./.reports/*_coverage.xml

pyproject.toml

@@ -5,6 +5,7 @@ description = "Django admin filter for multiple select"
 readme = "README.md"
 authors = [{ name = "Job Doesburg", email = "[email protected]" }]
 license = { file = "LICENSE" }
+requires-python = ">=3"
 classifiers = [
     "License :: OSI Approved :: MIT License",
     "Programming Language :: Python",
@@ -20,10 +21,15 @@ classifiers = [
     "Intended Audience :: Developers",
     "Operating System :: OS Independent",
 ]
+
 dependencies = [
     "django>=3",
 ]
-requires-python = ">=3"
+
+[project.optional-dependencies]
+django42 = ["django~=4.2.0"]
+django52 = ["django~=5.2.0"]
+test = ["pytest", "pytest-django", "factory_boy", "pytest-cov"]
 
 [project.urls]
 homepage = "https://github.com/JobDoesburg/django-admin-multi-select-filter"
@@ -39,3 +45,24 @@ where = ["src"]
 
 [tool.setuptools.package-data]
 "*" = ["*.html"]
+
+[tool.ruff]
+line-length = 119
+output-format = "grouped"
+show-fixes = true
+target-version = "py313"
+exclude = [".svn", "CVS", ".bzr", ".hg",".git", "__pycache__", ".tox", ".eggs", "*.egg", ".venv", "env", "venv", "build"]
+
+[tool.ruff.lint]
+select = ["W", "E", "F", "I", "N", "DJ", "T20", "Q"]
+
+[tool.ruff.lint.isort]
+combine-as-imports = true
+
+[tool.ruff.lint.mccabe]
+max-complexity = 6
+
+[tool.pytest.ini_options]
+DJANGO_SETTINGS_MODULE = "tests.app.settings"
+addopts = "-ra -q"
+pythonpath =". src"

src/django_admin_multi_select_filter/filters.py

@@ -14,39 +14,31 @@ def __init__(self, field, request, params, model, model_admin, field_path):
         super().__init__(field, request, params, model, model_admin, field_path)
 
         self.lookup_val = self.used_parameters.get(self.lookup_kwarg, [])
-        if len(self.lookup_val) == 1 and self.lookup_val[0] == "":
+        if len(self.lookup_val) == 1 and (self.lookup_val[0] == [""] or self.lookup_val[0] == ""):
             self.lookup_val = []
-        elif len(self.lookup_val) == 1 and type(self.lookup_val[0]) != str:
-            # In Django 5.0, we get an extra list
+        elif len(self.lookup_val) == 1 and not isinstance(self.lookup_val[0], str):
             self.lookup_val = self.lookup_val[0]
         self.lookup_val_isnull = self.used_parameters.get(self.lookup_kwarg_isnull)
 
         self.empty_value_display = model_admin.get_empty_value_display()
         parent_model, reverse_path = reverse_field_path(model, field_path)
-        # Obey parent ModelAdmin queryset when deciding which options to show
+
         if model == parent_model:
             queryset = model_admin.get_queryset(request)
         else:
             queryset = parent_model._default_manager.all()
-        self.lookup_choices = (
-            queryset.distinct().order_by(field.name).values_list(field.name, flat=True)
-        )
+        self.lookup_choices = queryset.distinct().order_by(field.name).values_list(field.name, flat=True)
         self.field_verboses = {}
         if self.field.choices:
-            self.field_verboses = {
-                field_value: field_verbose
-                for field_value, field_verbose in self.field.choices
-            }
+            self.field_verboses = {field_value: field_verbose for field_value, field_verbose in self.field.choices}
 
     def expected_parameters(self):
         return [self.lookup_kwarg, self.lookup_kwarg_isnull]
 
     def choices(self, changelist):
         yield {
             "selected": not self.lookup_val and self.lookup_val_isnull is None,
-            "query_string": changelist.get_query_string(
-                remove=[self.lookup_kwarg, self.lookup_kwarg_isnull]
-            ),
+            "query_string": changelist.get_query_string(remove=[self.lookup_kwarg, self.lookup_kwarg_isnull]),
             "display": _("All"),
         }
         include_none = False
@@ -73,18 +65,14 @@ def choices(self, changelist):
             else:
                 yield {
                     "selected": val in self.lookup_val,
-                    "query_string": changelist.get_query_string(
-                        remove=[self.lookup_kwarg]
-                    ),
+                    "query_string": changelist.get_query_string(remove=[self.lookup_kwarg]),
                     "display": self.field_verboses.get(val, val),
                 }
 
         if include_none:
             yield {
                 "selected": bool(self.lookup_val_isnull),
-                "query_string": changelist.get_query_string(
-                    {self.lookup_kwarg_isnull: "True"}, [self.lookup_kwarg]
-                ),
+                "query_string": changelist.get_query_string({self.lookup_kwarg_isnull: "True"}, [self.lookup_kwarg]),
                 "display": self.empty_value_display,
             }
 
@@ -94,20 +82,18 @@ def __init__(self, field, request, params, model, model_admin, field_path):
         super().__init__(field, request, params, model, model_admin, field_path)
         self.lookup_kwarg = "%s__%s__in" % (field_path, field.target_field.name)
         self.lookup_kwarg_isnull = "%s__isnull" % field_path
-        values = params.get(self.lookup_kwarg, [])
-        if len(values) == 1 and type(values[0]) != str:
-            # In Django 5.0, we get an extra list
-            values = values[0]
-        self.lookup_val = values.split(",") if values else []
+
+        values = request.GET.getlist(self.lookup_kwarg)
+        if len(values) == 1 and "," in values[0]:
+            values = values[0].split(",")
+        self.lookup_val = [str(value) for value in values if value]
+
         self.lookup_choices = self.field_choices(field, request, model_admin)
 
     def choices(self, changelist):
         yield {
-            "selected": (self.lookup_val is None or self.lookup_val == [])
-            and not self.lookup_val_isnull,
-            "query_string": changelist.get_query_string(
-                remove=[self.lookup_kwarg, self.lookup_kwarg_isnull]
-            ),
+            "selected": (self.lookup_val is None or self.lookup_val == []) and not self.lookup_val_isnull,
+            "query_string": changelist.get_query_string(remove=[self.lookup_kwarg, self.lookup_kwarg_isnull]),
             "display": _("All"),
         }
 
@@ -123,8 +109,7 @@ def choices(self, changelist):
                 values = self.lookup_val + [str(pk_val)]
 
             yield {
-                "selected": self.lookup_val is not None
-                and str(pk_val) in self.lookup_val,
+                "selected": self.lookup_val is not None and str(pk_val) in self.lookup_val,
                 "query_string": changelist.get_query_string(
                     {self.lookup_kwarg: ",".join(values)}, [self.lookup_kwarg_isnull]
                 ),
@@ -134,9 +119,7 @@ def choices(self, changelist):
         if self.include_empty_choice:
             yield {
                 "selected": bool(self.lookup_val_isnull),
-                "query_string": changelist.get_query_string(
-                    {self.lookup_kwarg_isnull: "True"}, [self.lookup_kwarg]
-                ),
+                "query_string": changelist.get_query_string({self.lookup_kwarg_isnull: "True"}, [self.lookup_kwarg]),
                 "display": empty_title,
             }
 
@@ -153,11 +136,7 @@ def queryset(self, request, queryset):
                 return queryset
 
             queryset = queryset.alias(
-                nmatch=Count(
-                    self.field_path,
-                    filter=Q(**{f'{self.lookup_kwarg}': choices}),
-                    distinct=True
-                )
+                nmatch=Count(self.field_path, filter=Q(**{f"{self.lookup_kwarg}": choices}), distinct=True)
             ).filter(nmatch=choice_len)
             return queryset
 

tests/__init__.py

@@ -0,0 +1,18 @@
+from django.contrib import admin
+
+from django_admin_multi_select_filter.filters import MultiSelectFieldListFilter, MultiSelectRelatedFieldListFilter
+from tests.app.models import Item, Tag
+
+
+@admin.register(Item)
+class ItemAdmin(admin.ModelAdmin):
+    list_display = ("name",)
+    list_filter = (
+        ("tags", MultiSelectRelatedFieldListFilter),
+        ("status", MultiSelectFieldListFilter),
+    )
+
+
+@admin.register(Tag)
+class TagAdmin(admin.ModelAdmin):
+    list_display = ("name",)

tests/app/__init__.py

@@ -0,0 +1,6 @@
+from django.apps import AppConfig
+
+
+class TestAppConfig(AppConfig):
+    name = "tests.app"
+    label = "testapp"

tests/app/admin.py

@@ -0,0 +1,26 @@
+from django.db import models
+
+
+class Tag(models.Model):
+    name = models.CharField(max_length=100)
+
+    def __str__(self):
+        return self.name
+
+
+class Item(models.Model):
+    name = models.CharField(max_length=100)
+    tags = models.ManyToManyField(Tag, blank=True)
+    status = models.CharField(
+        max_length=10,
+        choices=[
+            ("new", "New"),
+            ("old", "Old"),
+            ("archived", "Archived"),
+        ],
+        blank=True,
+        default="",
+    )
+
+    def __str__(self):
+        return self.name

tests/app/apps.py

@@ -0,0 +1,43 @@
+SECRET_KEY = "test"
+USE_TZ = True
+TIME_ZONE = "UTC"
+
+INSTALLED_APPS = [
+    "django.contrib.auth",
+    "django.contrib.contenttypes",
+    "django.contrib.sessions",
+    "django.contrib.messages",
+    "django.contrib.admin",
+    "tests.app",
+]
+
+MIDDLEWARE = [
+    "django.middleware.security.SecurityMiddleware",
+    "django.contrib.sessions.middleware.SessionMiddleware",
+    "django.middleware.common.CommonMiddleware",
+    "django.middleware.csrf.CsrfViewMiddleware",
+    "django.contrib.auth.middleware.AuthenticationMiddleware",
+    "django.contrib.messages.middleware.MessageMiddleware",
+]
+
+ROOT_URLCONF = "tests.app.urls"
+
+TEMPLATES = [
+    {
+        "BACKEND": "django.template.backends.django.DjangoTemplates",
+        "DIRS": [],
+        "APP_DIRS": True,
+        "OPTIONS": {
+            "context_processors": [
+                "django.template.context_processors.request",
+                "django.contrib.auth.context_processors.auth",
+                "django.contrib.messages.context_processors.messages",
+            ]
+        },
+    }
+]
+
+DATABASES = {"default": {"ENGINE": "django.db.backends.sqlite3", "NAME": ":memory:"}}
+STATIC_URL = "/static/"
+
+MIGRATION_MODULES = {"tests.app": None}