Skip to content
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
12 changes: 12 additions & 0 deletions openwisp_utils/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.core.exceptions import FieldError
from django.urls import reverse
from django.utils.translation import gettext_lazy as _
from openwisp_utils.widgets import Select2Widget


class TimeReadonlyAdminMixin(object):
Expand Down Expand Up @@ -217,3 +218,14 @@ def get_formset(self, request, obj=None, **kwargs):
formset = super().get_formset(request, obj, **kwargs)
formset.help_text = self.help_text
return formset


class Select2AdminMixin:
"""Mixin that applies Select2Widget to specified choice fields."""

select2_fields = ()

def formfield_for_choice_field(self, db_field, request, **kwargs):
if db_field.name in self.select2_fields:
kwargs["widget"] = Select2Widget()
return super().formfield_for_choice_field(db_field, request, **kwargs)
18 changes: 18 additions & 0 deletions openwisp_utils/static/openwisp-utils/js/select2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
"use strict";

django.jQuery(function ($) {
function initSelect2($element) {
$element.not('select[name*="__prefix__"]').each(function () {
var $el = $(this);
if (!$el.hasClass("select2-hidden-accessible")) {
$el.select2();
}
});
}

initSelect2($("select.ow-select2"));

$(document).on("formset:added", function (event, $row) {
initSelect2($row.find("select.ow-select2"));
});
Comment on lines +15 to +17
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect repository dependency constraints for supported Django versions.

set -euo pipefail

fd -H '^(setup\.py|setup\.cfg|pyproject\.toml|tox\.ini|requirements.*\.txt)$' \
  -x sh -c 'echo "### $1"; rg -n "Django|django" "$1" || true' sh {}

Repository: openwisp/openwisp-utils

Length of output: 595


🌐 Web query:

Django admin formset:added event signature event target row Django 4.1

💡 Result:

In Django 4.1 admin, the formset:added event is a JavaScript CustomEvent triggered when a new inline form row is added. Event signature: formset:added (dispatched on document). Event properties: - event.target: The newly added row element (HTMLElement). - event.detail.formsetName: The name of the formset (string). Usage example: document.addEventListener('formset:added', (event) => { if (event.detail.formsetName === 'my_formset') { // Access the new row const row = event.target; // Perform actions on the row } }); This changed in Django 4.1 from pre-4.1 jQuery events (event, $row, formsetName). For backward compatibility, check event.detail. Register in admin change_form.html by extending admin_change_form_document_ready block and including your JS.

Citations:


🏁 Script executed:

#!/bin/bash
# Search for the main Django version requirement in setup.py
cat setup.py | grep -A 5 -B 5 "install_requires" | head -30

Repository: openwisp/openwisp-utils

Length of output: 450


🏁 Script executed:

#!/bin/bash
# Check if Django version is specified anywhere else (requirements, tox, etc.)
rg "Django.*>=|Django.*<|django.*version" --no-heading

Repository: openwisp/openwisp-utils

Length of output: 684


🏁 Script executed:

#!/bin/bash
# Check the full select2.js file to see if there's any version handling or comments about Django
cat openwisp_utils/static/openwisp-utils/js/select2.js

Repository: openwisp/openwisp-utils

Length of output: 494


Normalize the added formset row before calling .find() for Django 4.1+ compatibility.

The formset:added event API changed in Django 4.1: the added row is now available via event.target as a CustomEvent, not as a second parameter ($row). The current code will fail with $row undefined and prevent Select2 initialization on dynamically added inline rows in Django 4.1+. The codebase supports Django >= 2.2 with no upper bound, so compatibility is required.

🐛 Proposed cross-version fix
-  $(document).on("formset:added", function (event, $row) {
-    initSelect2($row.find("select.ow-select2"));
+  $(document).on("formset:added", function (event, $row) {
+    var $targetRow = $row ? $($row) : $(event.target);
+    initSelect2($targetRow.find("select.ow-select2"));
   });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
$(document).on("formset:added", function (event, $row) {
initSelect2($row.find("select.ow-select2"));
});
$(document).on("formset:added", function (event, $row) {
var $targetRow = $row ? $($row) : $(event.target);
initSelect2($targetRow.find("select.ow-select2"));
});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openwisp_utils/static/openwisp-utils/js/select2.js` around lines 15 - 17, The
formset added handler in openwisp-utils/static/openwisp-utils/js/select2.js
assumes the added row is passed as the second argument ($row) but Django 4.1
emits a CustomEvent with the row at event.target; update the handler to
normalize the row variable before calling initSelect2—e.g., in the
$(document).on("formset:added", ...) callback determine rowEl = $row ? $row :
$(event.target) (or wrap event.target with $ if needed) and then call
initSelect2(rowEl.find("select.ow-select2")); ensure you reference the existing
event handler and initSelect2 function names so compatibility with Django >=2.2
and Django 4.1+ is preserved.

});
34 changes: 34 additions & 0 deletions openwisp_utils/widgets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
from django.conf import settings
from django.contrib.admin.widgets import SELECT2_TRANSLATIONS
from django.forms import Media, Select
from django.utils.translation import get_language


class Select2Widget(Select):
"""Select2 autocomplete widget for Django ChoiceFields."""

@property
def media(self):
extra = "" if getattr(settings, "DEBUG", False) else ".min"
i18n_name = SELECT2_TRANSLATIONS.get(get_language())
i18n_file = (
("admin/js/vendor/select2/i18n/{0}.js".format(i18n_name),)
if i18n_name
else ()
)
return Media(
js=(
"admin/js/vendor/jquery/jquery{0}.js".format(extra),
"admin/js/vendor/select2/select2.full{0}.js".format(extra),
)
+ i18n_file
+ ("admin/js/jquery.init.js", "openwisp-utils/js/select2.js"),
css={
"screen": ("admin/css/vendor/select2/select2{0}.css".format(extra),),
},
)

def __init__(self, attrs=None, choices=()):
attrs = attrs or {}
attrs["class"] = "ow-select2 {0}".format(attrs.get("class", "")).strip()
super().__init__(attrs, choices)
Comment on lines +31 to +34
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Avoid mutating caller-provided attrs in place.

Copy attrs first to prevent side effects when the same dict instance is reused.

Suggested refactor
     def __init__(self, attrs=None, choices=()):
-        attrs = attrs or {}
+        attrs = dict(attrs or {})
         attrs["class"] = "ow-select2 {0}".format(attrs.get("class", "")).strip()
         super().__init__(attrs, choices)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def __init__(self, attrs=None, choices=()):
attrs = attrs or {}
attrs["class"] = "ow-select2 {0}".format(attrs.get("class", "")).strip()
super().__init__(attrs, choices)
def __init__(self, attrs=None, choices=()):
attrs = dict(attrs or {})
attrs["class"] = "ow-select2 {0}".format(attrs.get("class", "")).strip()
super().__init__(attrs, choices)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openwisp_utils/widgets.py` around lines 31 - 34, The __init__ mutates the
caller-provided attrs dict in place; change it to copy attrs first (e.g., attrs
= dict(attrs) or attrs.copy() when attrs is truthy) before updating the "class"
key so the original dict is not modified; update the __init__ implementation
(the attrs handling and the super().__init__(attrs, choices) call) in the widget
class to use the copied dict.

4 changes: 3 additions & 1 deletion tests/test_project/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
HelpTextStackedInline,
ReadOnlyAdmin,
ReceiveUrlAdmin,
Select2AdminMixin,
TimeReadonlyAdminMixin,
UUIDAdmin,
)
Expand Down Expand Up @@ -110,7 +111,7 @@ class AutoOwnerFilter(AutocompleteFilter):


@admin.register(Shelf)
class ShelfAdmin(TimeReadonlyAdminMixin, admin.ModelAdmin):
class ShelfAdmin(Select2AdminMixin, TimeReadonlyAdminMixin, admin.ModelAdmin):
# DO NOT CHANGE: used for testing filters
list_filter = [
ShelfFilter,
Expand All @@ -121,6 +122,7 @@ class ShelfAdmin(TimeReadonlyAdminMixin, admin.ModelAdmin):
ReverseBookFilter,
]
search_fields = ["name"]
select2_fields = ("books_type",)


@admin.register(OrganizationRadiusSettings)
Expand Down
47 changes: 47 additions & 0 deletions tests/test_project/tests/test_widgets.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
from channels.testing import ChannelsLiveServerTestCase
from django.test import TestCase, tag
from django.urls import reverse
from openwisp_utils.widgets import Select2Widget
from selenium.webdriver.common.by import By

from ..models import Shelf
from .utils import SeleniumTestMixin


class TestWidgets(TestCase):
def test_select2_widget_attrs(self):
widget = Select2Widget()
html = widget.render("name", "value")
self.assertIn('class="ow-select2"', html)

# test overriding works and class is preserved
widget = Select2Widget(attrs={"class": "my-class"})
html = widget.render("name", "value")
self.assertIn("ow-select2 my-class", html)

def test_select2_widget_media(self):
widget = Select2Widget()
media = str(widget.media)
self.assertIn("admin/css/vendor/select2/select2", media)
self.assertIn("admin/js/vendor/jquery/jquery", media)
self.assertIn("admin/js/vendor/select2/select2.full", media)
self.assertIn("openwisp-utils/js/select2.js", media)


@tag("selenium_tests")
class TestSelect2AdminMixinSelenium(SeleniumTestMixin, ChannelsLiveServerTestCase):
def setUp(self):
super().setUp()
self.login()

def test_select2_widget_renders_on_shelf_add_form(self):
url = reverse("admin:test_project_shelf_add")
self.open(url)
self.wait_for_presence(By.CSS_SELECTOR, "select#id_books_type.ow-select2")

Comment on lines +37 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add-page Selenium test should also assert Select2 initialization, not just CSS class.

This test currently proves widget markup but not JS enhancement on add view. Please also assert the Select2 container is present on this path.

Suggested test tightening
     def test_select2_widget_renders_on_shelf_add_form(self):
         url = reverse("admin:test_project_shelf_add")
         self.open(url)
         self.wait_for_presence(By.CSS_SELECTOR, "select#id_books_type.ow-select2")
+        self.wait_for_presence(By.CSS_SELECTOR, ".select2-container")

Based on learnings: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_project/tests/test_widgets.py` around lines 37 - 41, The test
test_select2_widget_renders_on_shelf_add_form currently only checks for the
select element and its .ow-select2 class; update it to also assert the Select2
JS enhancement by waiting for the Select2 container for that field (for example
by waiting for a selector such as ".select2-container" or the field-specific
container tied to "id_books_type") to be present in the DOM after opening
reverse("admin:test_project_shelf_add"); use the existing helper
wait_for_presence to locate the Select2 container adjacent to or tied to the
select#id_books_type element to ensure initialization rather than just markup.

def test_select2_widget_renders_on_shelf_change_form(self):
shelf = Shelf.objects.create(name="Test Shelf", books_type="HORROR")
url = reverse("admin:test_project_shelf_change", args=[shelf.pk])
self.open(url)
self.wait_for_presence(By.CSS_SELECTOR, "select#id_books_type.ow-select2")
self.wait_for_presence(By.CSS_SELECTOR, ".select2-container")
Loading