[feat] Add Select2Widget for choice fields #254#642
[feat] Add Select2Widget for choice fields #254#642shivsubh wants to merge 1 commit intoopenwisp:masterfrom
Conversation
📝 WalkthroughWalkthroughAdds a reusable Select2 integration: a new Select2Widget (subclassing Django's Select) that injects Select2 assets and ensures the Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant DjangoAdmin as Django Admin
participant Widget as Select2Widget
participant JS as select2.js
participant Select2 as Select2 Library
User->>Browser: Request admin form page
Browser->>DjangoAdmin: GET /admin/... (render form)
DjangoAdmin->>Widget: Render select field with ow-select2 class
Widget->>Browser: Serve HTML + Media (CSS/JS)
Browser->>Browser: Load assets (jQuery, Select2, select2.js, i18n)
Browser->>JS: Execute initialization on DOM ready
JS->>JS: Find select.ow-select2 (exclude __prefix__)
JS->>Select2: Call $el.select2() for uninitialized elements
Select2->>Browser: Enhance UI (select2-container)
User->>Browser: Interact with enhanced select
sequenceDiagram
actor User
participant Browser
participant Form as Formset
participant JS as select2.js
participant Select2 as Select2 Library
User->>Form: Click "Add row"
Form->>Browser: Append new row and emit formset:added
Browser->>JS: formset:added handler runs
JS->>JS: Locate select.ow-select2 in new row (exclude __prefix__)
JS->>Select2: Initialize Select2 if not select2-hidden-accessible
Select2->>Browser: Enhance new select element
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge The implementation remains clean and follows Django best practices. No changes have been made to the PR files since the previous review. Incremental Review (latest commit)No changes to PR files since commit Files Reviewed (4 files)
Reviewed by kimi-k2.5-0127 · 103,448 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_project/tests/test_widgets.py`:
- Around line 17-23: The current test_select2_widget_media only checks default
asset inclusion; add two edge-case tests to cover minified assets and i18n:
create test_select2_widget_media_minified that patches
openwisp_utils.widgets.settings.DEBUG to False, instantiate Select2Widget and
assert the media string contains minified filenames (e.g., select2.min.css,
jquery.min.js, select2.full.min.js), and create test_select2_widget_media_i18n
that patches openwisp_utils.widgets.get_language to return a supported locale
like 'es', instantiate Select2Widget and assert the media string contains the
i18n path (e.g., i18n/es.js); use unittest.mock.patch to scope these changes and
reference Select2Widget and its media property in the assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9142d548-73bd-4704-b953-8e623f00fd20
📒 Files selected for processing (4)
openwisp_utils/static/openwisp-utils/js/select2.jsopenwisp_utils/widgets.pytests/test_project/admin.pytests/test_project/tests/test_widgets.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=5.2.0
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
tests/test_project/admin.pytests/test_project/tests/test_widgets.pyopenwisp_utils/widgets.py
🔇 Additional comments (4)
openwisp_utils/static/openwisp-utils/js/select2.js (1)
1-18: LGTM!The initialization script is well-structured with appropriate safeguards:
- Correctly excludes formset template rows via
__prefix__check- Idempotency guard using
select2-hidden-accessibleclass prevents double initialization- Proper handling of dynamically added formset rows via
formset:addedeventopenwisp_utils/widgets.py (1)
1-36: LGTM!The
Select2Widgetimplementation is clean and well-structured:
- Properly leverages Django's admin assets to avoid external dependencies
- Correctly handles DEBUG setting for minified/non-minified assets
- i18n support via
SELECT2_TRANSLATIONSgracefully handles missing translations- Asset ordering ensures dependencies load before initialization script
- Class attribute handling in
__init__correctly preserves user-provided classestests/test_project/admin.py (1)
126-129: LGTM!The
formfield_for_choice_fieldoverride correctly appliesSelect2Widgetonly to thebooks_typefield while preserving default behavior for other choice fields viasuper().tests/test_project/tests/test_widgets.py (1)
1-23: LGTM overall - tests cover core widget functionality.The test file verifies the essential behavior: CSS class handling and asset inclusion. The widget rendering and media composition are adequately tested for the basic use case.
| 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) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding test coverage for edge cases.
The current test_select2_widget_media test verifies asset inclusion but could be more comprehensive:
- No assertion for i18n file inclusion when
get_language()returns a supported language - No verification of minified vs non-minified asset paths based on
DEBUGsetting
💡 Suggested additional test cases
from unittest.mock import patch
def test_select2_widget_media_minified(self):
"""Test that minified assets are used when DEBUG=False."""
with patch('openwisp_utils.widgets.settings') as mock_settings:
mock_settings.DEBUG = False
widget = Select2Widget()
media = str(widget.media)
self.assertIn('select2.min.css', media)
self.assertIn('jquery.min.js', media)
self.assertIn('select2.full.min.js', media)
def test_select2_widget_media_i18n(self):
"""Test that i18n file is included for supported languages."""
with patch('openwisp_utils.widgets.get_language', return_value='es'):
widget = Select2Widget()
media = str(widget.media)
self.assertIn('i18n/es.js', media)🤖 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 17 - 23, The current
test_select2_widget_media only checks default asset inclusion; add two edge-case
tests to cover minified assets and i18n: create
test_select2_widget_media_minified that patches
openwisp_utils.widgets.settings.DEBUG to False, instantiate Select2Widget and
assert the media string contains minified filenames (e.g., select2.min.css,
jquery.min.js, select2.full.min.js), and create test_select2_widget_media_i18n
that patches openwisp_utils.widgets.get_language to return a supported locale
like 'es', instantiate Select2Widget and assert the media string contains the
i18n path (e.g., i18n/es.js); use unittest.mock.patch to scope these changes and
reference Select2Widget and its media property in the assertions.
shivsubh
force-pushed
the
issues/254-select2-autocomplete-fields
branch
from
a695791 to
7ec95e2
Compare
Code Style FailuresHello @shivsubh, Your commit failed due to multiple code style violations. Please run the Specifically, the following files have formatting errors that
Additionally, there are ReStructuredText (RST) formatting errors detected in |
shivsubh
force-pushed
the
issues/254-select2-autocomplete-fields
branch
2 times, most recently
from
fbf6555 to
780d3cb
Compare
|
coverage: 97.49% (+0.01%) from 97.476% — shivsubh:issues/254-select2-autocomplete-fields into openwisp:master |
pandafy
left a comment
pandafy
left a comment
There was a problem hiding this comment.
Thank you @shivsubh for contributing. We want to have a re-usable solution which means minimal code changes should be required in the downstream projects to utilize the new widget.
I did a quick review and found the following code problematic.
We will need new selenium tests which verifies the functioning of the added field.
github-project-automation
bot
moved this to To do (general)
in OpenWISP Contributor's Board
github-project-automation
bot
moved this from To do (general)
to In progress
in OpenWISP Contributor's Board
Implement a reusable Select2Widget that uses Django's native admin assets to avoid extra dependencies. Closes openwisp#254
shivsubh
force-pushed
the
issues/254-select2-autocomplete-fields
branch
from
780d3cb to
2ccf8d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@openwisp_utils/widgets.py`:
- Around line 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.
In `@tests/test_project/tests/test_widgets.py`:
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 736a54cb-1fcd-4b9c-b43f-6add1068f1cd
📒 Files selected for processing (5)
openwisp_utils/admin.pyopenwisp_utils/static/openwisp-utils/js/select2.jsopenwisp_utils/widgets.pytests/test_project/admin.pytests/test_project/tests/test_widgets.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.0.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.12 | django~=5.0.0
- GitHub Check: Python==3.10 | django~=5.0.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{py,html,txt}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
tests/test_project/admin.pyopenwisp_utils/admin.pytests/test_project/tests/test_widgets.pyopenwisp_utils/widgets.py
🧠 Learnings (2)
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Bug Fixes: If the bug affects the user interface, include a Selenium browser test; if missing, raise a warning
Applied to files:
tests/test_project/tests/test_widgets.py
📚 Learning: 2026-03-14T20:44:14.568Z
Learnt from: CR
Repo: openwisp/openwisp-utils PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-14T20:44:14.568Z
Learning: Features: Add tests for new features and ensure coverage does not decrease significantly; prefer Selenium browser tests for UI-impacting features
Applied to files:
tests/test_project/tests/test_widgets.py
🔇 Additional comments (5)
openwisp_utils/admin.py (1)
223-231: Clean reusable admin hook for Select2-enabled choice fields.This mixin-based opt-in is scoped and composable, and the
formfield_for_choice_fieldoverride keeps behavior limited to choice fields.tests/test_project/admin.py (1)
114-125: Test admin wiring is correct for feature coverage.Applying
Select2AdminMixinwithselect2_fields = ("books_type",)is the right setup to validate the feature end-to-end.openwisp_utils/static/openwisp-utils/js/select2.js (1)
4-17: Initialization flow handles both initial render and dynamic formsets well.The
__prefix__skip and already-initialized guard are good safeguards against duplicate/broken Select2 setup.tests/test_project/tests/test_widgets.py (1)
12-29: Widget-level tests are solid and focused.These assertions cover class behavior and media asset inclusion clearly.
openwisp_utils/widgets.py (1)
10-29: Media assembly is correct and complete for admin Select2 usage.The DEBUG-aware assets, optional i18n file, and custom initializer script are wired consistently.
| def __init__(self, attrs=None, choices=()): | ||
| attrs = attrs or {} | ||
| attrs["class"] = "ow-select2 {0}".format(attrs.get("class", "")).strip() | ||
| super().__init__(attrs, choices) |
There was a problem hiding this comment.
🧹 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.
| 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.
| 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") | ||
|
|
There was a problem hiding this comment.
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.
3 participants
Implement a reusable Select2Widget that uses Django's native admin assets to avoid extra dependencies.
Closes #254
Checklist
Reference to Existing Issue
Closes #254 .