Skip to content

Commit ce0cfd7

Browse files
pandafynemesifier
authored andcommitted
[fix] Fixed 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] (cherry picked from commit b73e83a)
1 parent 5f1971d commit ce0cfd7

File tree

3 files changed

+104
-4
lines changed

3 files changed

+104
-4
lines changed

openwisp_controller/config/static/config/js/relevant_templates.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ django.jQuery(function ($) {
138138
resetTemplateOptions();
139139
var enabledTemplates = [],
140140
sortedm2mUl = $("ul.sortedm2m-items:first"),
141-
sortedm2mPrefixUl = $("ul.sortedm2m-items:last");
141+
sortedm2mPrefixUl = $("#config-empty ul.sortedm2m-items");
142142

143143
// Adds "li" elements for templates
144144
Object.keys(data).forEach(function (templateId, index) {

openwisp_controller/config/tests/test_selenium.py

Lines changed: 98 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import time
22

3+
from django.contrib.auth.models import Group, Permission
34
from django.contrib.staticfiles.testing import StaticLiveServerTestCase
45
from django.test import tag
56
from django.urls.base import reverse
@@ -40,9 +41,17 @@ def _verify_templates_visibility(self, hidden=None, visible=None):
4041
hidden = hidden or []
4142
visible = visible or []
4243
for template in hidden:
43-
self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]')
44+
self.wait_for_invisibility(
45+
By.XPATH,
46+
f'//ul[contains(@class,"sortedm2m-items")]'
47+
f'//input[@value="{template.id}"]',
48+
)
4449
for template in visible:
45-
self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]')
50+
self.wait_for_visibility(
51+
By.XPATH,
52+
f'//ul[contains(@class,"sortedm2m-items")]'
53+
f'//input[@value="{template.id}"]',
54+
)
4655

4756

4857
@tag("selenium_tests")
@@ -373,6 +382,93 @@ def test_add_remove_templates(self):
373382
self.assertEqual(config.templates.count(), 0)
374383
self.assertEqual(config.status, "modified")
375384

385+
def test_relevant_templates_duplicates(self):
386+
"""
387+
Test that a user with specific permissions can see shared templates
388+
properly. Verifies that:
389+
1. User with custom group permissions can access the admin
390+
2. Multiple shared templates are displayed correctly
391+
3. Each template appears only once in the sortedm2m list
392+
"""
393+
# Define permission codenames for the custom group
394+
permission_codenames = [
395+
"view_group",
396+
"change_config",
397+
"view_config",
398+
"add_device",
399+
"change_device",
400+
"delete_device",
401+
"view_device",
402+
"view_devicegroup",
403+
"view_template",
404+
]
405+
# Create a custom group with the specified permissions
406+
permissions = Permission.objects.filter(codename__in=permission_codenames)
407+
custom_group, _ = Group.objects.get_or_create(name="Custom Operator")
408+
custom_group.permissions.set(permissions)
409+
# Create a user and assign the custom group
410+
user = self._create_user(
411+
username="limited_user",
412+
password="testpass123",
413+
email="limited@test.com",
414+
is_staff=True,
415+
)
416+
user.groups.add(custom_group)
417+
org = self._get_org()
418+
self._create_org_user(user=user, organization=org, is_admin=True)
419+
# Create multiple shared templates (organization=None)
420+
template1 = self._create_template(
421+
name="Shared Template 1", organization=None, default=True
422+
)
423+
template2 = self._create_template(name="Shared Template 2", organization=None)
424+
device = self._create_config(organization=org).device
425+
# Login as the limited user
426+
self.login(username="limited_user", password="testpass123")
427+
# Navigate using Selenium
428+
self.open(
429+
reverse("admin:config_device_change", args=[device.id]) + "#config-group"
430+
)
431+
self.hide_loading_overlay()
432+
with self.subTest(
433+
"Regression precondition: empty Config inline is not rendered"
434+
):
435+
self.assertFalse(self.web_driver.find_elements(By.ID, "config-empty"))
436+
437+
with self.subTest("All shared templates should be visible"):
438+
self._verify_templates_visibility(visible=[template1, template2])
439+
440+
with self.subTest("Verify sortedm2m list has exactly 2 template items"):
441+
# Check that ul.sortedm2m-items.sortedm2m.ui-sortable has exactly 2 children
442+
# with .sortedm2m-item class
443+
sortedm2m_items = self.find_elements(
444+
by=By.CSS_SELECTOR,
445+
value="ul.sortedm2m-items.sortedm2m.ui-sortable > li.sortedm2m-item",
446+
)
447+
self.assertEqual(
448+
len(sortedm2m_items),
449+
2,
450+
(
451+
"Expected exactly 2 template items in sortedm2m list,"
452+
f" found {len(sortedm2m_items)}"
453+
),
454+
)
455+
456+
with self.subTest(
457+
"Verify checkbox inputs are rendered with expected attributes"
458+
):
459+
for idx, template_id in enumerate([template1.id, template2.id]):
460+
checkbox = self.find_element(
461+
by=By.ID, value=f"id_config-templates_{idx}"
462+
)
463+
self.assertEqual(checkbox.get_attribute("value"), str(template_id))
464+
self.assertEqual(checkbox.get_attribute("data-required"), "false")
465+
466+
with self.subTest("Save operation completes successfully"):
467+
# Scroll to the top of the page to ensure the save button is visible
468+
self.web_driver.execute_script("window.scrollTo(0, 0);")
469+
self.find_element(by=By.NAME, value="_save").click()
470+
self.wait_for_presence(By.CSS_SELECTOR, ".messagelist .success", timeout=5)
471+
376472

377473
@tag("selenium_tests")
378474
class TestDeviceGroupAdmin(

openwisp_controller/geo/tests/test_selenium.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,11 @@ def setUp(self):
9191
def test_unsaved_changes_readonly(self):
9292
self.login()
9393
ol = self._create_object_location()
94-
path = reverse("admin:config_device_change", args=[ol.device.id])
94+
path = reverse(
95+
f"admin:{self.object_model._meta.app_label}_"
96+
f"{self.object_model._meta.model_name}_change",
97+
args=[ol.device.id],
98+
)
9599

96100
with self.subTest("Alert should not be displayed without any change"):
97101
self.open(path)

0 commit comments

Comments
 (0)