From 523f8ae5e953f756e9909212210024c6bf65b70c Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 20 Mar 2026 15:43:49 +0530 Subject: [PATCH 1/4] [fix] Fix duplicate template entries in Device admin Bug: The JS logic in relevant_templates.js assumed that the last `ul.sortedm2m-items` element belonged to the empty inline form used by Django admin formsets. This assumption breaks when the user does not have permission to add Config objects: in that case the ConfigInlineAdmin does not render the empty form. As a result, both selectors ended up referencing the same list and the script appended template elements twice to the same `sortedm2m` list, causing duplicate entries and issues when saving the form. Fix: Select the empty form container explicitly using `#config-empty ul.sortedm2m-items` instead of relying on the last occurrence of `ul.sortedm2m-items`. This ensures the logic works correctly regardless of whether the empty inline form is rendered. [backport 1.2] --- .../static/config/js/relevant_templates.js | 2 +- .../config/tests/test_selenium.py | 107 +++++++++++++++++- 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/openwisp_controller/config/static/config/js/relevant_templates.js b/openwisp_controller/config/static/config/js/relevant_templates.js index 9d7a1a766..72a1668ac 100644 --- a/openwisp_controller/config/static/config/js/relevant_templates.js +++ b/openwisp_controller/config/static/config/js/relevant_templates.js @@ -138,7 +138,7 @@ django.jQuery(function ($) { resetTemplateOptions(); var enabledTemplates = [], sortedm2mUl = $("ul.sortedm2m-items:first"), - sortedm2mPrefixUl = $("ul.sortedm2m-items:last"); + sortedm2mPrefixUl = $("#config-empty ul.sortedm2m-items"); // Adds "li" elements for templates Object.keys(data).forEach(function (templateId, index) { diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index f718b3373..c88891487 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -1,6 +1,7 @@ import os import time +from django.contrib.auth.models import Group, Permission from django.contrib.staticfiles.testing import StaticLiveServerTestCase from django.test import tag from django.urls.base import reverse @@ -45,9 +46,17 @@ def _verify_templates_visibility(self, hidden=None, visible=None): hidden = hidden or [] visible = visible or [] for template in hidden: - self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]') + self.wait_for_invisibility( + By.XPATH, + f'//ul[contains(@class,"sortedm2m-items")]' + f'//input[@value="{template.id}"]', + ) for template in visible: - self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]') + self.wait_for_visibility( + By.XPATH, + f'//ul[contains(@class,"sortedm2m-items")]' + f'//input[@value="{template.id}"]', + ) @tag("selenium_tests") @@ -389,6 +398,100 @@ def test_add_remove_templates(self): self.assertEqual(config.templates.count(), 0) self.assertEqual(config.status, "modified") + def test_relevant_templates_duplicates(self): + """ + Test that a user with specific permissions can see shared templates + properly. Verifies that: + 1. User with custom group permissions can access the admin + 2. Multiple shared templates are displayed correctly + 3. Each template appears only once in the sortedm2m list + 4. Default template is automatically selected + """ + # Define permission codenames for the custom group + permission_codenames = [ + "view_group", + "change_config", + "view_config", + "add_device", + "change_device", + "delete_device", + "view_device", + "view_devicegroup", + "view_template", + ] + # Create a custom group with the specified permissions + permissions = Permission.objects.filter(codename__in=permission_codenames) + custom_group, _ = Group.objects.get_or_create(name="Custom Operator") + custom_group.permissions.set(permissions) + # Create a user and assign the custom group + user = self._create_user( + username="limited_user", + password="testpass123", + email="limited@test.com", + is_staff=True, + ) + user.groups.add(custom_group) + org = self._get_org() + self._create_org_user(user=user, organization=org, is_admin=True) + # Create multiple shared templates (organization=None) + template1 = self._create_template( + name="Shared Template 1", organization=None, default=True + ) + template2 = self._create_template(name="Shared Template 2", organization=None) + device = self._create_config(organization=org).device + # Login as the limited user + self.login(username="limited_user", password="testpass123") + # Navigate using Selenium + self.open( + reverse("admin:config_device_change", args=[device.id]) + "#config-group" + ) + self.hide_loading_overlay() + with self.subTest("All shared templates should be visible"): + self._verify_templates_visibility(visible=[template1, template2]) + + with self.subTest("Verify sortedm2m list has exactly 2 template items"): + # Check that ul.sortedm2m-items.sortedm2m.ui-sortable has exactly 2 children + # with .sortedm2m-item class + sortedm2m_items = self.find_elements( + by=By.CSS_SELECTOR, + value="ul.sortedm2m-items.sortedm2m.ui-sortable > li.sortedm2m-item", + ) + self.assertEqual( + len(sortedm2m_items), + 2, + ( + "Expected exactly 2 template items in sortedm2m list," + f" found {len(sortedm2m_items)}" + ), + ) + + with self.subTest( + "Verify checkbox HTML elements are present in page source" + " with correct attributes" + ): + page_source = self.web_driver.page_source + # Verify HTML element strings for each template + for idx, template_id in enumerate([template1.id, template2.id], start=0): + html_element = ( + f' Date: Fri, 20 Mar 2026 16:46:54 +0530 Subject: [PATCH 2/4] [fix] Fixes by @coderabbitai --- .../config/tests/test_selenium.py | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index c88891487..27926a100 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -405,7 +405,6 @@ def test_relevant_templates_duplicates(self): 1. User with custom group permissions can access the admin 2. Multiple shared templates are displayed correctly 3. Each template appears only once in the sortedm2m list - 4. Default template is automatically selected """ # Define permission codenames for the custom group permission_codenames = [ @@ -466,25 +465,14 @@ def test_relevant_templates_duplicates(self): ) with self.subTest( - "Verify checkbox HTML elements are present in page source" - " with correct attributes" + "Verify checkbox inputs are rendered with expected attributes" ): - page_source = self.web_driver.page_source - # Verify HTML element strings for each template - for idx, template_id in enumerate([template1.id, template2.id], start=0): - html_element = ( - f' Date: Fri, 27 Mar 2026 09:26:03 +0530 Subject: [PATCH 3/4] [fix] Fixes by @coderabbitai --- openwisp_controller/config/tests/test_selenium.py | 3 ++- openwisp_controller/geo/tests/test_selenium.py | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index 27926a100..7bdc799b9 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -442,7 +442,8 @@ def test_relevant_templates_duplicates(self): self.login(username="limited_user", password="testpass123") # Navigate using Selenium self.open( - reverse("admin:config_device_change", args=[device.id]) + "#config-group" + reverse(f"admin:{self.config_app_label}_device_change", args=[device.id]) + + "#config-group" ) self.hide_loading_overlay() with self.subTest("All shared templates should be visible"): diff --git a/openwisp_controller/geo/tests/test_selenium.py b/openwisp_controller/geo/tests/test_selenium.py index 03d1dcac0..1a24354c9 100644 --- a/openwisp_controller/geo/tests/test_selenium.py +++ b/openwisp_controller/geo/tests/test_selenium.py @@ -91,7 +91,11 @@ def setUp(self): def test_unsaved_changes_readonly(self): self.login() ol = self._create_object_location() - path = reverse("admin:config_device_change", args=[ol.device.id]) + path = reverse( + f"admin:{self.object_model._meta.app_label}_" + f"{self.object_model._meta.model_name}_change", + args=[ol.device.id], + ) with self.subTest("Alert should not be displayed without any change"): self.open(path) From 4c9e834004256f0a4a32f0005dd41de9347959fa Mon Sep 17 00:00:00 2001 From: Gagan Deep Date: Fri, 27 Mar 2026 15:25:27 +0530 Subject: [PATCH 4/4] [fix] Fixed tests --- openwisp_controller/config/tests/test_selenium.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/openwisp_controller/config/tests/test_selenium.py b/openwisp_controller/config/tests/test_selenium.py index 7bdc799b9..4322c00c5 100644 --- a/openwisp_controller/config/tests/test_selenium.py +++ b/openwisp_controller/config/tests/test_selenium.py @@ -446,6 +446,11 @@ def test_relevant_templates_duplicates(self): + "#config-group" ) self.hide_loading_overlay() + with self.subTest( + "Regression precondition: empty Config inline is not rendered" + ): + self.assertFalse(self.web_driver.find_elements(By.ID, "config-empty")) + with self.subTest("All shared templates should be visible"): self._verify_templates_visibility(visible=[template1, template2])