Skip to content

Commit b73e83a

Browse files
authored
[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]
1 parent af0d99b commit b73e83a

3 files changed

Lines changed: 105 additions & 4 deletions

File tree

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: 99 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import os
22
import time
33

4+
from django.contrib.auth.models import Group, Permission
45
from django.contrib.staticfiles.testing import StaticLiveServerTestCase
56
from django.test import tag
67
from django.urls.base import reverse
@@ -45,9 +46,17 @@ def _verify_templates_visibility(self, hidden=None, visible=None):
4546
hidden = hidden or []
4647
visible = visible or []
4748
for template in hidden:
48-
self.wait_for_invisibility(By.XPATH, f'//*[@value="{template.id}"]')
49+
self.wait_for_invisibility(
50+
By.XPATH,
51+
f'//ul[contains(@class,"sortedm2m-items")]'
52+
f'//input[@value="{template.id}"]',
53+
)
4954
for template in visible:
50-
self.wait_for_visibility(By.XPATH, f'//*[@value="{template.id}"]')
55+
self.wait_for_visibility(
56+
By.XPATH,
57+
f'//ul[contains(@class,"sortedm2m-items")]'
58+
f'//input[@value="{template.id}"]',
59+
)
5160

5261

5362
@tag("selenium_tests")
@@ -389,6 +398,94 @@ def test_add_remove_templates(self):
389398
self.assertEqual(config.templates.count(), 0)
390399
self.assertEqual(config.status, "modified")
391400

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

393490
@tag("selenium_tests")
394491
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)